On Wed, 6 Jan 2016, Terry Moore wrote:
You may well be right. From looking at the man page, fd_getfile() assumes the the file is already open. Is there an additional spec_write() after the fd_getfile()? I don't see it in you patch.
spec_write() is called via the dereferencing at the end of filemon_output() ...
In any case, I was using writabality as an example. It's really fragile to depend on grabbing a file handle and assuming it's what you had before. The security analysis of that is essentially open-ended -- and has to be revisited every time the behavior of files as seen by fd_getfile() changes [therefore is an eternal burden], whereas the analysis of adding an additional exit hook is trivial, and as far as I can see, never has to be revisited.
Point taken. In this case, write access is checked on each call, so that's not a problem. But without holding the fd_getfile() reference, the application program is indeed frre to switch things out from under us. I've attempted to minimize that by comparing the pointers to the 'struct file' but it doesn't guarantee that things have not changed.
One more reason for us to retain the extra reference as originally written, and then modify exithooks as previously proposed to clean things up in the correct order.
I understand everyone wanting to be conservative about not adding new facilities to the kernel, but sometimes a new facility actually saves a lot of effort overall. You're doing the work, so having made my point, I'll let you make your decision. --Terry-----Original Message----- From: tech-kern-owner%NetBSD.org@localhost [mailto:tech-kern-owner%NetBSD.org@localhost] On Behalf Of Paul Goyette Sent: Wednesday, January 6, 2016 21:31 To: Terry Moore <tmm%mcci.com@localhost> Cc: tech-kern%netbsd.org@localhost Subject: RE: In-kernel process exit hooks? I'm pretty sure that the mode check done at the beginning of spec_write() will ensure that the file is opened with write access. :) On Wed, 6 Jan 2016, Terry Moore wrote:Isn't there a security risk with the fd_getfile() approach? This sounds (on the face of it) similar to the kinds of problems that led tmpnam(3) to be deprecated? For example, what if the monitoring program deliberately points the fd at a file that it opened as read-only;will filemon then write to it?--Terry-----Original Message----- From: tech-kern-owner%NetBSD.org@localhost [mailto:tech-kern-owner%NetBSD.org@localhost] On Behalf Of Paul Goyette Sent: Wednesday, January 6, 2016 16:55 To: Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> Cc: tech-kern%netbsd.org@localhost Subject: Re: In-kernel process exit hooks?Another possibility would be to change filemon(4) to do fd_getfile each it needs to use the file descriptor. This makes it a little more brittle (fails if you close the descriptor), but would sidestep the problem.Hmmm, perhaps. Failure would not be a problem, since we would just revert to the initial "output file unspecified" conditions. I think I like this approach. :) I'll give it a try.This actually works quite well. Please see the attached diffs for your review. One possible problem is what happens if the monitoring program closes the file descriptor, and then re-uses that fd? I've included a check to compare the original 'struct file *' pointer with the current one, which will catch "some" instances, but not guaranteed to catch them all. It could be a bit of a surprise if filemon output shows up in unexpected places. :) Because of this potential for surprising the user, I think I'm still leaning to my earlier proposal of extending exithook processing. But given the limited number of use-cases for filemon, I could live with making the fd_getfile()-only-when-you-need-it change instead. +------------------+--------------------------+------------------------+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at | netbsd.org | +------------------+--------------------------+------------------------++------------------+--------------------------+------------------------+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org | +------------------+--------------------------+------------------------+
+------------------+--------------------------+------------------------+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org | +------------------+--------------------------+------------------------+