Re: [PATCH] inotify: fix one-shot support

[Ingo Oeser]

Hi Robert,
hi John,

just saw this commit.

On Wednesday 08 February 2006 02:05, you wrote:
> tree 5b5af4e03e627b66a9f37d25dd370a145ec72438
> parent 8e08b756869eeb08ace17ad64c2a8cb97b18e856
> author Robert Love <rml@xxxxxxxxxx> Wed, 08 Feb 2006 04:58:45 -0800
> committer Linus Torvalds <torvalds@xxxxxxxxxxx> Wed, 08 Feb 2006 08:12:33 -0800
> 
> [PATCH] inotify: fix one-shot support
> 
> Fix one-shot support in inotify.  We currently drop the IN_ONESHOT flag
> during watch addition.  Fix is to not do that.
 
Yes, but now you can add a watch without any event attached. 
This would revert the original sense of the test.

> Signed-off-by: Robert Love <rml@xxxxxxxxxx>
> Cc: John McCutchan <ttb@xxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx>
> 
>  fs/inotify.c |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/inotify.c b/fs/inotify.c
> index 878ccca..3041503 100644
> --- a/fs/inotify.c
> +++ b/fs/inotify.c
> @@ -967,7 +967,7 @@ asmlinkage long sys_inotify_add_watch(in
>  		mask_add = 1;
>  
>  	/* don't let user-space set invalid bits: we don't want flags set */
> -	mask &= IN_ALL_EVENTS;
> +	mask &= IN_ALL_EVENTS | IN_ONESHOT;
>  	if (unlikely(!mask)) {
>  		ret = -EINVAL;
>  		goto out;

See, now you can just pass IN_ONESHOT behavior flag without any
events to shoot at, which you couldn't do before. But this makes only
sense, if we would like to set a multi-shot mask to one-shot now.

Does this transition (multi shot to single shot)makes sense at all? 
Is it race-free to allow this?.

So my suggested fix instead of yours would be:

/* don't let user-space set invalid bits: we don't want flags set */
mask &= IN_ALL_EVENTS | IN_ONESHOT;
if (unlikely((mask & IN_ALL_EVENTS) == 0 && !mask_add)) {
	ret = -EINVAL;
	goto out;
}

Would you like a patch on top of the one submitted by you?


Regards

Ingo Oeser

Attachment: pgp00000.pgp
Description: PGP signature