Page MenuHomeFreeBSD

inotify: Unconditionally generate IN_IGNORED events for files/dirs
ClosedPublic

Authored by markj on Mon, Jul 6, 1:00 AM.

Details

Summary

The implementation previously only generated an IN_IGNORED event for a
deleted watched file if the watch explicitly requested IN_DELETE_SELF.
This is not correct, IN_IGNORED should always be raised when the watched
subject is deleted. Adjust the implementation of inotify_log_one()
accordingly.

This also fixes a problem where a deleted watched file's watch
would not be removed if IN_DELETE_SELF was not in the watch's event
mask, in which case the unlinked vnode would linger until the inotify
descriptor itself is closed.

Add a regression test.

Reported by: jrtc27

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Mon, Jul 6, 1:00 AM

@jrtc27 note that IN_IGNORED and IN_UNMOUNT aren't events that you explicitly ask for when you call inotify_add_watch(), they're generated automatically.

sys/kern/vfs_inotify.c
630

This preserves the current behaviour of IN_UNMOUNT being sent regardless of whether you asked for it, but is that actually meant to be the case? I don't see anything in Linux's manpage that would suggest that, and my comments on IRC were that this looked odd. AFAIUI unmount is just like delete, in that the only special behaviour it has is (a) triggering an IN_IGNORED if you've asked for that (b) removing it from the rb tree. But I have not empirically verified that.

@jrtc27 note that IN_IGNORED and IN_UNMOUNT aren't events that you explicitly ask for when you call inotify_add_watch(), they're generated automatically.

Uh, I think you do? I don't think you're meant to get them if not in the mask. The only special behaviour for IN_IGNORED is that it's triggered by the same thing as IN_UNMOUNT and IN_DELETE_SELF, regardless of whether you're listening for the latter?

The untested https://termbin.com/tmhd is what my mental model is from reading the manpage and thinking about what sensible semantics would be. Maybe it should follow the existing locking discipline as in this patch, though I don't obviously see why that complexity is important for slightly shrinking the lock duration.

@jrtc27 note that IN_IGNORED and IN_UNMOUNT aren't events that you explicitly ask for when you call inotify_add_watch(), they're generated automatically.

Uh, I think you do? I don't think you're meant to get them if not in the mask.

No, you're supposed to get them no matter what you asked for. The Linux man page describes them as flags set by the kernel in returned events. Note also that these flags don't belong to IN_ALL_EVENTS.

@jrtc27 note that IN_IGNORED and IN_UNMOUNT aren't events that you explicitly ask for when you call inotify_add_watch(), they're generated automatically.

Uh, I think you do? I don't think you're meant to get them if not in the mask.

No, you're supposed to get them no matter what you asked for. The Linux man page describes them as flags set by the kernel in returned events. Note also that these flags don't belong to IN_ALL_EVENTS.

Huh, I see. The comments in the Linux kernel header are way clearer on that front; the inotify manpages are a bit of a mess that don't explain how it all fits together. I was just reading the headers and they say this about IN_UNMOUNT/Q_OVERFLOW/IGNORED:

the following are legal events. they are sent as needed to any watch

This revision is now accepted and ready to land.Mon, Jul 6, 1:28 AM