Re: [PATCH v2 09/16] fanotify: allow to set errno in FAN_DENY permission response

Josef Bacik
2024-08-09T18:38:42Z

On Fri, Aug 09, 2024 at 02:06:56PM +0200, Christian Brauner wrote:
> On Thu, Aug 08, 2024 at 03:27:11PM GMT, Josef Bacik wrote:
> > From: Amir Goldstein <amir73il@gmail.com>
> >
> > With FAN_DENY response, user trying to perform the filesystem operation
> > gets an error with errno set to EPERM.
> >
> > It is useful for hierarchical storage management (HSM) service to be able
> > to deny access for reasons more diverse than EPERM, for example EAGAIN,
> > if HSM could retry the operation later.
> >
> > Allow fanotify groups with priority FAN_CLASSS_PRE_CONTENT to responsd
> > to permission events with the response value FAN_DENY_ERRNO(errno),
> > instead of FAN_DENY to return a custom error.
> >
> > Limit custom error values to errors expected on read(2)/write(2) and
> > open(2) of regular files. This list could be extended in the future.
> > Userspace can test for legitimate values of FAN_DENY_ERRNO(errno) by
> > writing a response to an fanotify group fd with a value of FAN_NOFD in
> > the fd field of the response.
> >
> > The change in fanotify_response is backward compatible, because errno is
> > written in the high 8 bits of the 32bit response field and old kernels
> > reject respose value with high bits set.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> > fs/notify/fanotify/fanotify.c | 18 ++++++++++-----
> > fs/notify/fanotify/fanotify.h | 10 +++++++++
> > fs/notify/fanotify/fanotify_user.c | 36 +++++++++++++++++++++++++-----
> > include/linux/fanotify.h | 5 ++++-
> > include/uapi/linux/fanotify.h | 7 ++++++
> > 5 files changed, 65 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 4e8dce39fa8f..1cbf41b34080 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -224,7 +224,8 @@ static int fanotify_get_response(struct fsnotify_group *group,
> > struct fanotify_perm_event *event,
> > struct fsnotify_iter_info *iter_info)
> > {
> > - int ret;
> > + int ret, errno;
> > + u32 decision;
> >
> > pr_debug("%s: group=%p event=%p\n", __func__, group, event);
> >
> > @@ -257,20 +258,27 @@ static int fanotify_get_response(struct fsnotify_group *group,
> > goto out;
> > }
> >
> > + decision = fanotify_get_response_decision(event->response);
> > /* userspace responded, convert to something usable */
> > - switch (event->response & FANOTIFY_RESPONSE_ACCESS) {
> > + switch (decision & FANOTIFY_RESPONSE_ACCESS) {
> > case FAN_ALLOW:
> > ret = 0;
> > break;
> > case FAN_DENY:
> > + /* Check custom errno from pre-content events */
> > + errno = fanotify_get_response_errno(event->response);
>
> Fwiw, you're fetching from event->response again but have already
> stashed it in @decision earlier. Probably just an oversight.
>

Decision is the part that has the errno masked off, event->response is the full
mask which will have the errno set in the upper bits, so we have to do the
separate call with event->response to get the errno. Thanks,

Josef