On Fri, Oct 15, 2021 at 12:39 AM Gabriel Krisman Bertazi
<krisman@xxxxxxxxxxxxx> wrote:
>
> Non-inode errors will reported with an empty file_handle. In
> preparation for that, allow some events to print the FID record even if
> there isn't any file_handle encoded
>
> Even though FILEID_ROOT is used internally, make zero-length file
> handles be reported as FILEID_INVALID.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify_user.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 5324890500fc..39cf8ba4a6ce 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -127,6 +127,16 @@ static int fanotify_fid_info_len(int fh_len, int name_len)
> FANOTIFY_EVENT_ALIGN);
> }
>
> +static bool fanotify_event_allows_empty_fh(struct fanotify_event *event)
> +{
> + switch (event->type) {
> + case FANOTIFY_EVENT_TYPE_FS_ERROR:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static size_t fanotify_event_len(unsigned int info_mode,
> struct fanotify_event *event)
> {
> @@ -157,7 +167,7 @@ static size_t fanotify_event_len(unsigned int info_mode,
> if (info_mode & FAN_REPORT_PIDFD)
> event_len += FANOTIFY_PIDFD_INFO_HDR_LEN;
>
> - if (fh_len)
> + if (fh_len || fanotify_event_allows_empty_fh(event))
> event_len += fanotify_fid_info_len(fh_len, dot_len);
>
> return event_len;
> @@ -338,9 +348,6 @@ static int copy_fid_info_to_user(__kernel_fsid_t *fsid, struct fanotify_fh *fh,
> pr_debug("%s: fh_len=%zu name_len=%zu, info_len=%zu, count=%zu\n",
> __func__, fh_len, name_len, info_len, count);
>
> - if (!fh_len)
> - return 0;
> -
> if (WARN_ON_ONCE(len < sizeof(info) || len > count))
> return -EFAULT;
>
> @@ -375,6 +382,11 @@ static int copy_fid_info_to_user(__kernel_fsid_t *fsid, struct fanotify_fh *fh,
>
> handle.handle_type = fh->type;
> handle.handle_bytes = fh_len;
> +
> + /* Mangle handle_type for bad file_handle */
> + if (!fh_len)
> + handle.handle_type = FILEID_INVALID;
> +
> if (copy_to_user(buf, &handle, sizeof(handle)))
> return -EFAULT;
>
> @@ -467,7 +479,8 @@ static int copy_info_records_to_user(struct fanotify_event *event,
> total_bytes += ret;
> }
>
> - if (fanotify_event_object_fh_len(event)) {
> + if (fanotify_event_object_fh_len(event) ||
> + fanotify_event_allows_empty_fh(event)) {
> const char *dot = NULL;
> int dot_len = 0;
>
I don't like this fanotify_event_allows_empty_fh() implementation so much.
How about this instead:
static inline struct fanotify_fh *fanotify_event_object_fh(
struct fanotify_event *event)
{
struct fanotify_fh *fh = NULL;
/* An error event encodes (a FILEID_INVAL) fh for an empty fh */
if (event->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
return &FANOTIFY_EE(event)->object_fh;
else if (event->type == FANOTIFY_EVENT_TYPE_FID)
fh = &FANOTIFY_FE(event)->object_fh;
else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
fh = fanotify_info_file_fh(&FANOTIFY_NE(event)->info);
if (!fh && !fh->len)
return NULL;
return fh;
}
struct fanotify_fh *object_fh = fanotify_event_object_fh(event);
...
- if (fanotify_event_object_fh_len(event)) {
+ if (object_fh) {
const char *dot = NULL;
...
ret = copy_fid_info_to_user(fanotify_event_fsid(event),
- fanotify_event_object_fh(event),
+ object_fh,
info_type, dot, dot_len,
buf, count);
...
And similar change to fanotify_event_len()
This way, the logic of whether to report fh or not is encoded in
fanotify_event_object_fh() and fanotify_event_object_fh_len()
goes back to being a property of the the fh report.
Thanks,
Amir.
