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

Christian Brauner
2024-08-09T12:07:01Z

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.

> + 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
>