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. > In process_server_svc, an assumption is made that all uses of > inetd as a restarter will be with the default inetd FMRI. I'm not > sure that this is good logic. Hmm... interesting point. I'd argue this isn't a problem because I consider inspecting the inetd property groups to be a special case -- a courtesy, as it were, extended to delegates of inetd. If you want to hang your services on a different restarter, be it another instance of inetd (which I'm not sure is even supported) or something else, you can specify a firewall_context property group. Rather than try to extend this to other things that might behave like inetd, I'd prefer that the restarter itself include a definition of where its instances' firewall-specific data is. This would eliminate the need for the special case entirely. But it is also completely unnecessary at this point in time. If we ever encounter a need for it, we can add support for it to the proposed mechanism without breaking compatibility. Dave
