2014-04-23 21:56:47

by Heinrich Schuchardt

Subject: [PATCH 1/1] fanotify: for FAN_MARK_FLUSH check flags

If fanotify_mark is called with illegal value of arguments flags and marks
it usually returns EINVAL.

When fanotify_mark is called with FAN_MARK_FLUSH the argument flags is not
checked for irrelevant flags like FAN_MARK_IGNORED_MASK.

The patch removes this inconsistency.

If an irrelevant flag is set error EINVAL is returned.

Signed-off-by: Heinrich Schuchardt <[email protected]>
---
fs/notify/fanotify/fanotify_user.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 287a22c..8bba549 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -819,7 +819,10 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
case FAN_MARK_REMOVE:
if (!mask)
return -EINVAL;
+ break;
case FAN_MARK_FLUSH:
+ if (flags & ~(FAN_MARK_MOUNT | FAN_MARK_FLUSH))
+ return -EINVAL;
break;
default:
return -EINVAL;
--
1.9.2

2014-04-24 05:38:30

by Michael Kerrisk (man-pages)

Subject: Re: [PATCH 1/1] fanotify: for FAN_MARK_FLUSH check flags

On 04/23/2014 11:55 PM, Heinrich Schuchardt wrote:
> If fanotify_mark is called with illegal value of arguments flags and marks
> it usually returns EINVAL.
>
> When fanotify_mark is called with FAN_MARK_FLUSH the argument flags is not
> checked for irrelevant flags like FAN_MARK_IGNORED_MASK.
>
> The patch removes this inconsistency.
>
> If an irrelevant flag is set error EINVAL is returned.
>
> Signed-off-by: Heinrich Schuchardt <[email protected]>


So, a small heads up that this change may of course break existing code.
(Heinrich, see https://lwn.net/Articles/588444/ ). However,
there is some precedent for such changes (examples in
https://lwn.net/Articles/588444/), and

* The number of applications out there using fanotify is probably very low
* The number that are using FAN_MARK_FLUSH and misusing the flags will be even lower

So the risk of breakage is likely vanishingly small. So, a qualified

Acked-by: Michael Kerrisk <[email protected]>

Cheers,

Michael

> ---
> fs/notify/fanotify/fanotify_user.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 287a22c..8bba549 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -819,7 +819,10 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> case FAN_MARK_REMOVE:
> if (!mask)
> return -EINVAL;
> + break;
> case FAN_MARK_FLUSH:
> + if (flags & ~(FAN_MARK_MOUNT | FAN_MARK_FLUSH))
> + return -EINVAL;
> break;
> default:
> return -EINVAL;
>


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

2014-04-24 10:03:32

by Jan Kara

Subject: Re: [PATCH 1/1] fanotify: for FAN_MARK_FLUSH check flags

On Wed 23-04-14 23:55:51, Heinrich Schuchardt wrote:
> If fanotify_mark is called with illegal value of arguments flags and marks
> it usually returns EINVAL.
>
> When fanotify_mark is called with FAN_MARK_FLUSH the argument flags is not
> checked for irrelevant flags like FAN_MARK_IGNORED_MASK.
>
> The patch removes this inconsistency.
>
> If an irrelevant flag is set error EINVAL is returned.
OK, as Michael I think this shouldn't cause real userspace breakage and
it's better to have the flags checked. So feel free to add:
Acked-by: Jan Kara <[email protected]>

Honza

>
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
> fs/notify/fanotify/fanotify_user.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 287a22c..8bba549 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -819,7 +819,10 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> case FAN_MARK_REMOVE:
> if (!mask)
> return -EINVAL;
> + break;
> case FAN_MARK_FLUSH:
> + if (flags & ~(FAN_MARK_MOUNT | FAN_MARK_FLUSH))
> + return -EINVAL;
> break;
> default:
> return -EINVAL;
> --
> 1.9.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR