On Thursday 28 Oct 2010 22:32:32 Eric Paris wrote:
> fanotify has a defualt max queue depth. This patch allows processes which
> explicitly request it to have an 'unlimited' queue depth. These processes
> need to be very careful to make sure they cannot fall far enough behind
> that they OOM the box. Thus this flag is gated on CAP_SYS_ADMIN.
>
> Signed-off-by: Eric Paris <[email protected]>
> ---
>
> fs/notify/fanotify/fanotify_user.c | 9 ++++++++-
> include/linux/fanotify.h | 5 +++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c
> b/fs/notify/fanotify/fanotify_user.c index 04f2fe4..43d66d9 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -691,7 +691,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags,
> unsigned int, event_f_flags) goto out_put_group;
> }
>
> - group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS;
> + if (flags & FAN_UNLIMITED_QUEUE) {
> + fd = -EPERM;
> + if (!capable(CAP_SYS_ADMIN))
> + goto out_put_group;
Either this capable call is not needed or the one at the top of the syscall
needs to go if you intended to allow non-privileged access.
Tvrtko
Sophos Plc, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 348 3873 20.
On Mon, 2010-11-01 at 17:09 +0000, Tvrtko Ursulin wrote:
> On Thursday 28 Oct 2010 22:32:32 Eric Paris wrote:
> > fanotify has a defualt max queue depth. This patch allows processes which
> > explicitly request it to have an 'unlimited' queue depth. These processes
> > need to be very careful to make sure they cannot fall far enough behind
> > that they OOM the box. Thus this flag is gated on CAP_SYS_ADMIN.
> >
> > Signed-off-by: Eric Paris <[email protected]>
> > ---
> >
> > fs/notify/fanotify/fanotify_user.c | 9 ++++++++-
> > include/linux/fanotify.h | 5 +++--
> > 2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/notify/fanotify/fanotify_user.c
> > b/fs/notify/fanotify/fanotify_user.c index 04f2fe4..43d66d9 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -691,7 +691,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags,
> > unsigned int, event_f_flags) goto out_put_group;
> > }
> >
> > - group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS;
> > + if (flags & FAN_UNLIMITED_QUEUE) {
> > + fd = -EPERM;
> > + if (!capable(CAP_SYS_ADMIN))
> > + goto out_put_group;
>
> Either this capable call is not needed or the one at the top of the syscall
> needs to go if you intended to allow non-privileged access.
I realize they are redundant. But it is starting down the path of
unpriv'd users. I figure I already have to go back and audit everything
we do looking for places we shouldn't allow unpriv users, so no need to
add new ones without explicitly checking.
-Eric
On Monday 01 Nov 2010 17:23:51 Eric Paris wrote:
> On Mon, 2010-11-01 at 17:09 +0000, Tvrtko Ursulin wrote:
> > On Thursday 28 Oct 2010 22:32:32 Eric Paris wrote:
> > > fanotify has a defualt max queue depth. This patch allows processes
> > > which explicitly request it to have an 'unlimited' queue depth. These
> > > processes need to be very careful to make sure they cannot fall far
> > > enough behind that they OOM the box. Thus this flag is gated on
> > > CAP_SYS_ADMIN.
> > >
> > > Signed-off-by: Eric Paris <[email protected]>
> > > ---
> > >
> > > fs/notify/fanotify/fanotify_user.c | 9 ++++++++-
> > > include/linux/fanotify.h | 5 +++--
> > > 2 files changed, 11 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/notify/fanotify/fanotify_user.c
> > > b/fs/notify/fanotify/fanotify_user.c index 04f2fe4..43d66d9 100644
> > > --- a/fs/notify/fanotify/fanotify_user.c
> > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > @@ -691,7 +691,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int,
> > > flags, unsigned int, event_f_flags) goto out_put_group;
> > >
> > > }
> > >
> > > - group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS;
> > > + if (flags & FAN_UNLIMITED_QUEUE) {
> > > + fd = -EPERM;
> > > + if (!capable(CAP_SYS_ADMIN))
> > > + goto out_put_group;
> >
> > Either this capable call is not needed or the one at the top of the
> > syscall needs to go if you intended to allow non-privileged access.
>
> I realize they are redundant. But it is starting down the path of
> unpriv'd users. I figure I already have to go back and audit everything
> we do looking for places we shouldn't allow unpriv users, so no need to
> add new ones without explicitly checking.
True, next patch mentions this higher motive, sorry for the noise.
Tvrtko
Sophos Plc, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 348 3873 20.