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