New: inotify_rm_watch(2) unspecified behavior
https://bugzilla.kernel.org/show_bug.cgi?id=76851
Bug ID: 76851
Summary: inotify_rm_watch(2) unspecified behavior
Product: Documentation
Version: unspecified
Hardware: All
OS: Linux
Status: NEW
Severity: normal
Priority: P1
Component: man-pages
Assignee: documentation_man-pages-ztI5WcYan/[email protected]
Reporter: [email protected]
Regression: No
The inotify interface leaves unspecified what happens to potentially "pending" watch events (i.e., caught by the kernel but not yet read()) after an inotify_rm_watch() call.
IN_IGNORE flagged events *allow* inotify_rm_watch() calls to be monitored asynchronously, but it is not stated whether triggering actions between a most recent read() and an inotify_rm_watch() call *require* all other watch events to be handled so when users key map inotify_event wd fields to local objects.
Similarly, a conservative user would need to keep a queue of removed objects formerly associated with a watch descriptor if it is assumed possible that the kernel can both deliver events on an rm'd wd and reuse wds before the last pertinent event has been read.
If the purpose of IN_IGNORE being generated on the watch removal is to protect against poll/blocking-read vs. inotify_rm_watch() races, it should be plainly stated. Likewise, if it is needed for safely reacting to "stale" events, this should be explained. I believe that the current language of inotify_rm_watch(2) and the read(2) section of inotify(7) leave both interpretations defensible, which is sub-optimal.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #1 from Jeff <[email protected]> --- For what it's worth, neither Documentation/filesystems/inotify.txt nor fs/notify/inotify/inotify_user.c itself in the kernel source plainly comment on the issue. As far as I can tell, nothing besides the implementation standardizes the behavior.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
Michael Kerrisk <[email protected]> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |[email protected]
--- Comment #2 from Michael Kerrisk <[email protected]> --- (In reply to Jeff from comment #0)
Quoted 15 lines
The inotify interface leaves unspecified what happens to potentially "pending" watch events (i.e., caught by the kernel but not yet
read()) after aninotify_rm_watch()call.
IN_IGNOREflagged events *allow*inotify_rm_watch()calls to be monitored asynchronously, but it is not stated whether triggering actions between a most recentread()and aninotify_rm_watch()call *require* all other watch events to be handled so when users key mapinotify_eventwd fields to local objects.Similarly, a conservative user would need to keep a queue of removed objects formerly associated with a watch descriptor if it is assumed possible that the kernel can both deliver events on an rm'd wd and reuse wds before the last pertinent event has been read.
Quoted 6 lines
If the purpose of
IN_IGNOREbeing generated on the watch removal is to protect against poll/blocking-read vs.inotify_rm_watch()races, it should be plainly stated. Likewise, if it is needed for safely reacting to "stale" events, this should be explained. I believe that the current language ofinotify_rm_watch(2)and the read(2) section of inotify(7) leave both interpretations defensible, which is sub-optimal.
Jeff, thanks for the report. You're quite correct that there's no spec for the behavior (and your points about the implications of the lack of such spec are right on the money). However, man-pages is (sadly) not a spec, at least given current development practices. Nevertheless, something should be documented.
One thing that would have been useful in the bug report would have been a statement of what the existing behavior actually is. As far as I can see, as things stand, if a watch is removed, pending unread events for that watch descriptor remain pending. Can you confirm?
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #3 from Jeff <[email protected]> --- (In reply to Michael Kerrisk from comment #2)
Quoted 10 lines
Jeff, thanks for the report. You're quite correct that there's no spec for the behavior (and your points about the implications of the lack of such spec are right on the money). However, man-pages is (sadly) not a spec, at least given current development practices. Nevertheless, something should be documented.
One thing that would have been useful in the bug report would have been a statement of what the existing behavior actually is. As far as I can see, as things stand, if a watch is removed, pending unread events for that watch descriptor remain pending. Can you confirm?
Michael, I can confirm that this behavior, for at least 3.x kernels. I didn't test this or dig through the kernel source for anything older though.
I've not been involved enough in the kernel scene to know best how to approach this, so far as claiming what exactly seemed wrong, but the documentation seemed like the most innocuous reasonable place to start. I frankly don't even know where "standard" system call APIs are supposed to be officially specified, if anywhere.
I tried to be dispassionate in my initial report since I don't know what peoples' intents were, but it's my opinion that the current behavior is bad enough to warrant changing. I've never used any other multiplexing interface that can deliver events after a handle de-registration. I've also never seen anyone's inotify-handling code other than my own that even tried to safely handle late queued updates, and I suspect that some fraction of those people are ignorant of the hazards.
As I tried to imply above, extant IN_IGNORE event generation would provide sufficient safety from client poll/rm/read deadlock races in the event of an implementation change, but I don't have the slightest clue about tactfully approaching the kernel community with this, being a non-established individual.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #4 from Jeff <[email protected]> --- I should also add that my assumptions about watch descriptor reuse may very well be overly cautious. If there is a guarantee that a wd will never be returned by a inotify_add_watch() call until nothing with that wd remains in the read queue, clients could handle deregistration semi-synchronously by knowing to drop locally unmapped events.
This is something that I've not investigated recently, and I am completely uncertain of any guarantees, but the kernel folks may have already done this and just not passed the information along to users.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
Heinrich Schuchardt <[email protected]> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |[email protected]
--- Comment #5 from Heinrich Schuchardt <[email protected]> --- Created attachment 137681
--> https://bugzilla.kernel.org/attachment.cgi?id=137681&action=edittest_wd_reuse.c - Test watch descriptor reuse
The test program demonstrates that inotify_add_watch may return a watch descriptor ID for which events still exist on the inotify queue.
It creates one watch for file "0" to demonstrate the problem. Afterwards it creates and removes watches again and again just to force idr_alloc_cyclic to reach INT_MAX.
Then events are created for file "0" on the queue.
The watch for "0" is removed.
A watch for another file is created.
Example program output (after a few hours, on Linux 3.14.4 x86_64):
...
2147459044 2147467235 2147475426 2147483617 2147483647
Preparation done
BINGO Collision detected
Watch descriptor 1 for /tmp/test/8192
Watch descriptor 1 for /tmp/test/0
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #6 from Heinrich Schuchardt <[email protected]> --- Watch descriptor IDs are returned by inotify_add_watch.
When calling inotify_rm_watch an IN_IGNORE is placed on the inotify queue pointing to the ID of the removed watch.
inotify_add_watch should not return a watch descriptor ID for which events are still on the queue but should return an unused ID.
Unfortunately the existing Kernel code does not provide such a guarantee.
Actually in rare cases watch descriptor IDs are returned by inotify_add_watch for which events are still on the inotify queue.
I verified the problem exists with the appended test program (test_wd_reuse.c).
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #7 from Michael Kerrisk <[email protected]> --- (In reply to Jeff from comment #3)
Hi Jeff,
Quoted 6 lines
I tried to be dispassionate in my initial report since I don't know what peoples' intents were, but it's my opinion that the current behavior is bad enough to warrant changing. I've never used any other multiplexing interface that can deliver events after a handle de-registration. I've also never seen anyone's inotify-handling code other than my own that even tried to safely handle late queued updates,
(I have some code that handles it.)
Quoted 2 lines
and I suspect that some fraction of those people
are ignorant of the hazards.
I expect you are right,
Quoted 5 lines
As I tried to imply above, extant
IN_IGNOREevent generation would provide sufficient safety from client poll/rm/read deadlock races in the event of an implementation change, but I don't have the slightest clue about tactfully approaching the kernel community with this, being a non-established individual.
I'm not sure I understand the sentence "As I tried to imply...", but I'm guessing you mean that it would be sufficient to discard all pending events except the IN_IGNORED event, right?
It depends on the use case, I guess, some folk would, I expect, want to do log tracking -- that is, record all events that happened on a file. Such applications wouldn't want events to be discarded.
Cheers,
Michael
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #8 from Michael Kerrisk <[email protected]> --- (In reply to Jeff from comment #4)
Quoted 4 lines
I should also add that my assumptions about watch descriptor reuse may very well be overly cautious. If there is a guarantee that a wd will never be returned by a
inotify_add_watch()call until nothing with that wd remains in the read queue [...]
And that's the nub of the issue of course. As I suspected, and Heinrich has demonstrated in #5, the kernel does not provide such a guarantee.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 76851] inotify_rm_watch(2) unspecified behavior
In reply to a message not in the archivehttps://bugzilla.kernel.org/show_bug.cgi?id=76851
--- Comment #9 from Michael Kerrisk <[email protected]> --- (In reply to Heinrich Schuchardt from comment #5)
Quoted 15 lines
Created attachment 137681 [details]
test_wd_reuse.c- Test watch descriptor reuseThe test program demonstrates that
inotify_add_watchmay return a watch descriptor ID for which events still exist on the inotify queue.It creates one watch for file "0" to demonstrate the problem. Afterwards it creates and removes watches again and again just to force
idr_alloc_cyclicto reachINT_MAX.
Then events are created for file "0" on the queue.
The watch for "0" is removed.
A watch for another file is created.Example program output (after a few hours, on Linux 3.14.4
x86_64):
Quoted 7 lines
...
2147459044 2147467235 2147475426 2147483617 2147483647Preparation done
BINGO Collision detected
Watch descriptor 1 for/tmp/test/8192
Watch descriptor 1 for/tmp/test/0
Nice work Heinrich! I pretty much suspected this would be the case, even before looking at the kernel code, and it's nice to see a demonstration[1] of the problem. (I had in mind to code something similar myself, just to be sure, but you beat me to it.)
Anyway, this recycling behavior, coupled with "events remain pending after inotify_rm_watch()" behavior that Jeff reports mean that yes indeed we can events on a recycled watch descriptor that do indeed belong to a previous incarnation of that WD. On the other hand, you have to work pretty hard to do
this: you have to fail to read your queued events while at the same time cycling through INT_MAX watch descriptors. I suppose no application is going to run into that scenario in a hurry.
Cheers,
Michael
[1] No need to wait for hours when you test, Just rebuild a kernel with the following edit in fs/notify/inotify/inotify_user.c::inotify_add_to_idr()
- ret = idr_alloc_cyclic(idr, i_mark, 1, 0, GFP_NOWAIT);
- ret = idr_alloc_cyclic(idr, i_mark, 1, 500000, GFP_NOWAIT);
That's sufficient as a proof of concept of the problem.
Signature · 6 lines
You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html