[prev in list] [next in list] [prev in thread] [next in thread] 

List:       opensolaris-networking-discuss
Subject:    Re: [networking-discuss] [smf-discuss] Code review request
From:       Darren Reed <Darren.Reed () Sun ! COM>
Date:       2008-12-24 1:01:45
Message-ID: 495189F9.5010705 () Sun ! COM
[Download RAW message or body]

David Powell wrote:
> Darren Reed wrote:
>> David Powell wrote:
>>> Roland Mainz wrote:
>>>> David Powell wrote:
>>>>>      ipf_get_lock is racy.  A simpler and more robust lock file 
>>>>> can be
>>>>>      created by creating a lockfile.$newpid that contains $newpid and
>>>>>      attempting to ln $IPF_LOCK to your lockfile.
>>>>>       
>>>> Erm... AFAIK the only portable way (portable across all types of
>>>> filesystems, including NFS and DFS) is to use the atomic nature of
>>>> "mkdir", e.g. use "mkdir" to create the lock and "rmdir" to release 
>>>> it,
>>>> if "mkdir" fails then spin at this lock in incremetally larger time
>>>> intervals (e.g 0.1, 0.2, 0.4, 0.8, 1.6 seconds etc.).
>>>>     
>>>
>>>    In this case it only ever needs to work on tmpfs.
>>
>> The filesystem makes no difference.
>>
>> Whether or not the permissions on the owning directory are 777
>> or not changes whether or not it is a serious security problem.
>
>   /var/run has permissions 755.
>
>> But I think there are other problems with this file...
>>
>> If we consider that all SMF instances can have multiple instances
>> then all of the configuration file settings in this script need to be
>> changable because it is highly unlikely that the non-default instances
>> will want to use the same configuration files or the same lock file
>> path name.
>
>   The configuration file names include the name of the instance.
>
>   The lock file is a global lock file.
>
>> To answer that concern, it might be appropriate for ipf_include.sh
>> to attempt to get all of the configuration filenames from service
>> properties and if they don't exist (or are NULL) then apply a
>> default name... that may also require an update of the PSARC
>> case to document new private properties...*may*
>
>   Temporary configuration file names should be automatically generated
>   by the framework, period.  There's no benefit, and considerable
>   burden, to requiring the service author to coordinate and define
>   temporary configuration file names.

I was referring to lines 27-35 and 39 of ipf_include.sh

So.....

If anyone ever created svc:/network/ipfilter:foo, it would be
impossible to tie that together with svc:/network/ssh:foo because
this script would never match the ipfilter FMRI in the ssh service
with the correct one... but that might be pointless because it is
not possible to use svc:/network/ipfilter with anything other than
the default paths, anyway. So I suspect my concern is not all
that important.

Darren

_______________________________________________
networking-discuss mailing list
networking-discuss@opensolaris.org
[prev in list] [next in list] [prev in thread] [next in thread]