On Wed 04-08-21 12:06:08, Gabriel Krisman Bertazi wrote:
> Plumb the pieces to add a FID report to error records. Since all error
> event memory must be pre-allocated, we estimate a file handler size and
> if it is insuficient, we report an invalid FID and increase the
> prediction for the next error slot allocation.
Hum, cannot we just preallocate MAX_HANDLE_SZ? It is wasteful but then I
don't expect error marks would be that frequent for this to really
matter and the code would be simpler.
Honza
>
> For errors that don't expose a file handler report it with an invalid
> FID.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 27 +++++++++++++++++++++++++++
> fs/notify/fanotify/fanotify.h | 12 ++++++++++++
> fs/notify/fanotify/fanotify_user.c | 19 ++++++++++++++++++-
> 3 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 4e9e271a4394..2fd30b5eb9d3 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -705,7 +705,9 @@ static void fanotify_insert_error_event(struct fsnotify_group *group,
> {
> const struct fs_error_report *report = (struct fs_error_report *) data;
> struct fanotify_event *fae = FANOTIFY_E(event);
> + struct inode *inode = report->inode;
> struct fanotify_error_event *fee;
> + int fh_len;
>
> /* This might be an unexpected type of event (i.e. overflow). */
> if (!fanotify_is_error_event(fae->mask))
> @@ -715,6 +717,31 @@ static void fanotify_insert_error_event(struct fsnotify_group *group,
> fee->fae.type = FANOTIFY_EVENT_TYPE_FS_ERROR;
> fee->error = report->error;
> fee->err_count = 1;
> + fee->fsid = fee->sb_mark->fsn_mark.connector->fsid;
> +
> + /*
> + * Error reporting needs to happen in atomic context. If this
> + * inode's file handler length is more than we initially
> + * predicted, there is nothing better we can do than report the
> + * error with a bad file handler.
> + */
> + fh_len = fanotify_encode_fh_len(inode);
> + if (fh_len > fee->sb_mark->pred_fh_len) {
> + pr_warn_ratelimited(
> + "FH overflows error event. Drop inode information.\n");
> + /*
> + * Update the handler size prediction for the next error
> + * event allocation. This reduces the chance of another
> + * overflow.
> + */
> + fee->sb_mark->pred_fh_len = fh_len;
> +
> + /* For the current error, ignore the inode information. */
> + inode = NULL;
> + fh_len = fanotify_encode_fh_len(NULL);
> + }
> +
> + fanotify_encode_fh(&fee->object_fh, inode, fh_len, NULL, 0);
> }
>
> /*
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 8929ea50f96f..e4e7968b70ee 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -142,6 +142,7 @@ FANOTIFY_MARK_FLAG(SB_MARK);
>
> struct fanotify_sb_mark {
> struct fsnotify_mark fsn_mark;
> + size_t pred_fh_len;
> struct fanotify_error_event *fee_slot;
> };
>
> @@ -227,6 +228,13 @@ struct fanotify_error_event {
> u32 err_count; /* Suppressed errors count */
>
> struct fanotify_sb_mark *sb_mark; /* Back reference to the mark. */
> +
> + __kernel_fsid_t fsid; /* FSID this error refers to. */
> + /*
> + * object_fh is followed by a variable sized buffer, so it must
> + * be the last element of this structure.
> + */
> + struct fanotify_fh object_fh;
> };
>
> static inline struct fanotify_error_event *
> @@ -241,6 +249,8 @@ static inline __kernel_fsid_t *fanotify_event_fsid(struct fanotify_event *event)
> return &FANOTIFY_FE(event)->fsid;
> else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
> return &FANOTIFY_NE(event)->fsid;
> + else if (event->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
> + return &FANOTIFY_EE(event)->fsid;
> else
> return NULL;
> }
> @@ -252,6 +262,8 @@ static inline struct fanotify_fh *fanotify_event_object_fh(
> return &FANOTIFY_FE(event)->object_fh;
> else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
> return fanotify_info_file_fh(&FANOTIFY_NE(event)->info);
> + else if (event->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
> + return &FANOTIFY_EE(event)->object_fh;
> else
> return NULL;
> }
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index e7fe6bc61b6f..74def82630bb 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -172,8 +172,25 @@ static struct fanotify_error_event *fanotify_alloc_error_event(
>
> {
> struct fanotify_error_event *fee;
> + struct super_block *sb;
>
> - fee = kzalloc(sizeof(*fee), GFP_KERNEL_ACCOUNT);
> + if (!sb_mark->pred_fh_len) {
> + /*
> + * The handler size is initially predicted to be the
> + * same size as the root inode file handler. It can be
> + * increased later if a larger file handler is found.
> + */
> + sb = container_of(sb_mark->fsn_mark.connector->obj,
> + struct super_block, s_fsnotify_marks);
> + sb_mark->pred_fh_len =
> + fanotify_encode_fh_len(sb->s_root->d_inode);
> + }
> +
> + /* Guarantee there is always space to report an invalid FID. */
> + if (sb_mark->pred_fh_len < FANOTIFY_NULL_FH_LEN)
> + sb_mark->pred_fh_len = FANOTIFY_NULL_FH_LEN;
> +
> + fee = kzalloc(sizeof(*fee) + sb_mark->pred_fh_len, GFP_KERNEL_ACCOUNT);
> if (!fee)
> return NULL;
>
> --
> 2.32.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
