On Thu, Aug 08, 2024 at 03:27:11PM GMT, Josef Bacik wrote:
> From: Amir Goldstein <amir7...@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 <amir7...@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.

> +             if (errno) {
> +                     ret = -errno;
> +                     break;
> +             }
> +             fallthrough;
>       default:
>               ret = -EPERM;
>       }
>  
>       /* Check if the response should be audited */
> -     if (event->response & FAN_AUDIT)
> -             audit_fanotify(event->response & ~FAN_AUDIT,
> -                            &event->audit_rule);
> +     if (decision & FAN_AUDIT)
> +             audit_fanotify(decision & ~FAN_AUDIT, &event->audit_rule);
>  
>       pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
>                group, event, ret);
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 7f06355afa1f..d0722ef13138 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -528,3 +528,13 @@ static inline unsigned int 
> fanotify_mark_user_flags(struct fsnotify_mark *mark)
>  
>       return mflags;
>  }
> +
> +static inline u32 fanotify_get_response_decision(u32 res)
> +{
> +     return res & (FANOTIFY_RESPONSE_ACCESS | FANOTIFY_RESPONSE_FLAGS);
> +}
> +
> +static inline int fanotify_get_response_errno(int res)
> +{
> +     return res >> FAN_ERRNO_SHIFT;
> +}
> diff --git a/fs/notify/fanotify/fanotify_user.c 
> b/fs/notify/fanotify/fanotify_user.c
> index ed56fe6f5ec7..0a37f1c761aa 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -337,11 +337,13 @@ static int process_access_response(struct 
> fsnotify_group *group,
>       struct fanotify_perm_event *event;
>       int fd = response_struct->fd;
>       u32 response = response_struct->response;
> +     u32 decision = fanotify_get_response_decision(response);
> +     int errno = fanotify_get_response_errno(response);
>       int ret = info_len;
>       struct fanotify_response_info_audit_rule friar;
>  
> -     pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%zu\n", __func__,
> -              group, fd, response, info, info_len);
> +     pr_debug("%s: group=%p fd=%d response=%x errno=%d buf=%p size=%zu\n",
> +              __func__, group, fd, response, errno, info, info_len);
>       /*
>        * make sure the response is valid, if invalid we do nothing and either
>        * userspace can send a valid response or we will clean it up after the
> @@ -350,18 +352,42 @@ static int process_access_response(struct 
> fsnotify_group *group,
>       if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
>               return -EINVAL;
>  
> -     switch (response & FANOTIFY_RESPONSE_ACCESS) {
> +     switch (decision & FANOTIFY_RESPONSE_ACCESS) {
>       case FAN_ALLOW:
> +             if (errno)
> +                     return -EINVAL;
> +             break;
>       case FAN_DENY:
> +             /* Custom errno is supported only for pre-content groups */
> +             if (errno && group->priority != FSNOTIFY_PRIO_PRE_CONTENT)
> +                     return -EINVAL;
> +
> +             /*
> +              * Limit errno to values expected on open(2)/read(2)/write(2)
> +              * of regular files.
> +              */
> +             switch (errno) {
> +             case 0:
> +             case EIO:
> +             case EPERM:
> +             case EBUSY:
> +             case ETXTBSY:
> +             case EAGAIN:
> +             case ENOSPC:
> +             case EDQUOT:
> +                     break;
> +             default:
> +                     return -EINVAL;
> +             }
>               break;
>       default:
>               return -EINVAL;
>       }
>  
> -     if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
> +     if ((decision & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
>               return -EINVAL;
>  
> -     if (response & FAN_INFO) {
> +     if (decision & FAN_INFO) {
>               ret = process_access_response_info(info, info_len, &friar);
>               if (ret < 0)
>                       return ret;
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index ae6cb2688d52..547514542669 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -132,7 +132,10 @@
>  /* These masks check for invalid bits in permission responses. */
>  #define FANOTIFY_RESPONSE_ACCESS (FAN_ALLOW | FAN_DENY)
>  #define FANOTIFY_RESPONSE_FLAGS (FAN_AUDIT | FAN_INFO)
> -#define FANOTIFY_RESPONSE_VALID_MASK (FANOTIFY_RESPONSE_ACCESS | 
> FANOTIFY_RESPONSE_FLAGS)
> +#define FANOTIFY_RESPONSE_ERRNO      (FAN_ERRNO_MASK << FAN_ERRNO_SHIFT)
> +#define FANOTIFY_RESPONSE_VALID_MASK \
> +     (FANOTIFY_RESPONSE_ACCESS | FANOTIFY_RESPONSE_FLAGS | \
> +      FANOTIFY_RESPONSE_ERRNO)
>  
>  /* Do not use these old uapi constants internally */
>  #undef FAN_ALL_CLASS_BITS
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index cc28dce5f744..7b746c5fcbd8 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -233,6 +233,13 @@ struct fanotify_response_info_audit_rule {
>  /* Legit userspace responses to a _PERM event */
>  #define FAN_ALLOW    0x01
>  #define FAN_DENY     0x02
> +/* errno other than EPERM can specified in upper byte of deny response */
> +#define FAN_ERRNO_BITS       8
> +#define FAN_ERRNO_SHIFT (32 - FAN_ERRNO_BITS)
> +#define FAN_ERRNO_MASK       ((1 << FAN_ERRNO_BITS) - 1)
> +#define FAN_DENY_ERRNO(err) \
> +     (FAN_DENY | ((((__u32)(err)) & FAN_ERRNO_MASK) << FAN_ERRNO_SHIFT))
> +
>  #define FAN_AUDIT    0x10    /* Bitmask to create audit record for result */
>  #define FAN_INFO     0x20    /* Bitmask to indicate additional information */
>  
> -- 
> 2.43.0
> 

Reply via email to