directory change notification patch

Jeremy Allison jra at samba.org
Sat Mar 26 02:17:47 GMT 2005


On Fri, Mar 25, 2005 at 11:37:30PM +0000, Mark Weaver wrote:
> >>Sorry, I'm a bit confused now.  Does this mean that there is already a 
> >>patch that does the same thing?  I couldn't find anything, other than 
> >>the original that I based my changes off -- from Feb 2003 -- and 
> >
> >
> >Yes, it's that one. It's been outstanding a long time :-(.
> >
> I'm sure that there have been plenty of other things to take care of!
> 
> Here's another version of the patch with the following changes:
> 
> * Hopefully resolve security issue opening directories in 
> kernel_notify.c by opening them and then becoming root before issuing 
> the DNOTIFY call.  I believe that this covers the security issue (checks 
> that user can access directory), and also covers the lost signals issue 
> (signals are always delivered if they were registered for as root).
> * Fix a missing free in change_notify_set in an error path
> * Fix missing check for chdir() failure when processing notifications
> * Fix potential restart failure, then cnotify->remove_notify usage 
> (which would have caused a segmentation fault)
> * Add parameter to configuration file: 'change notify tracking' which 
> can be set to none|disk|memory.  none implements the old behaviour of 
> always returning STATUS_NOTIFY_ENUM_DIR.  This is the default option, 
> and is sufficient to make explorer work.  This could be changed, but as 
> it is the patch has minimal impact on existing installation.  memory 
> uses a memory backed tdb for storing directory contents, disk uses a 
> disk backed tdb.  The memory and disk options provide a performance for 
> space trade-off.
> 
> I've had it running for a few days without problems, and put it through 
> a few hoops.  Let me know if there are any other changes required (or bugs!)

Ok, a few comments. You're changing lib/select.c. This makes me very
nervous. This is critical code, tested on millions of systems and took
us a long time to get right. Are you sure your changes are correct ? Why
do you need them ?

Secondly, you're mixing direct POSIX calls (eg. open()) with Samba VFS
calls. You can't do that. All calls touching the filesystem *must* go
via the VFS calls.

Just my first thoughts as I looked over it.

Jeremy.


More information about the samba-technical mailing list