On Wed, 2009-07-01 at 09:41 -0400, Stephen Smalley wrote: > On Tue, 2009-06-30 at 16:57 -0400, Daniel J Walsh wrote: > > On 06/30/2009 03:47 PM, Stephen Smalley wrote: > > > On Tue, 2009-06-30 at 15:32 -0400, Thomas Liu wrote: > > >> This patch converted setfiles/restorecon to using fts instead of nftw. > > >> It also removed forking, pipes and pre_stat because Dan Walsh and I > > >> could not figure out what it was for. > > > > > > See: > > > http://marc.info/?l=selinux&m=113627973615236&w=2 > > > for the rationale for that logic. > > But this does not seem to be much benefit since fts files come with the > > stat already filled out. Thomas removed all stat/lstat calls since you > > are not allowed to use the -D_FILE_OFFSET_BITS=64 with fts. > > I'm open to considering the change, but I'd like to see actual > measurements before and after the changes. Also, I'd like to see the output of a setfiles -v for an entire system before and after the patch to verify that there is no change in behavior on existing systems, i.e. that no label gets changed as a result of the patch. > > I'd also like answers to the questions I posed regarding use of fts_path > vs. fts_accpath (I think we should be using the latter for calls to > getfilecon and setfilecon at least), whether the removal of the extra > lstat call changes behavior for symlinks (nftw would pass the stat > struct of the referenced file rather than the symlink, and the man page > for fts suggests that it may do likewise, which may change how we end up > labeling symlinks due to the different modes), and whether you looked at > the FreeBSD setfsmac.c implementation to see if we can learn anything > useful from it. > > > > Did you run any measurements to assess the impact of your changes on setfiles? > > > > > I think the biggest impact on speed comes from not trying to read past a > > failed directory. > > > > One thing that would be helpful to add would be to check if a directory > > is on a files system that supports labeling, if not then the directory > > should be skipped. I think with this patch the tool would walk a nfs_t > > file system unless it is called as setfiles. > > > Did you compare with the implementation of setfsmac.c in FreeBSD, which > > > likewise started life as setfiles and was then rewritten to use fts()? > > > > > > What improvements do we get from the changes? > > > > > > Should we be using fts_path or fts_accpath when performing operations on > > > the files, like lsetfilecon()? I suspect the latter would be more > > > efficient and less prone to simultaneous changes to the file tree if fts > > > switches the cwd as it walks the tree. > > > > > > IIRC, we called lstat() within apply_spec() even though a struct stat > > > was supplied as an argument because in the case of symlinks, we were > > > getting the stat of the referenced file rather than of the symlink file. > > > Is this true of fts? > > > > > > > > > -- > > This message was distributed to subscribers of the selinux mailing list. > > If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with > > the words "unsubscribe selinux" without quotes as the message. -- Stephen Smalley National Security Agency -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.
