Thread (61 messages) 61 messages, 4 authors, 2021-10-18

Re: [PATCH v7 23/28] fanotify: Report fid info for file related file system errors

From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-10-15 07:56:51
Also in: linux-ext4, linux-fsdevel

On Fri, Oct 15, 2021 at 12:39 AM Gabriel Krisman Bertazi
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
Plumb the pieces to add a FID report to error records.  Since all error
event memory must be pre-allocated, we pre-allocate the maximum file
handle size possible, such that it should always fit.

For errors that don't expose a file handle report it with an invalid
FID.

Signed-off-by: Gabriel Krisman Bertazi <redacted>

---
Changes since v6:
  - pass fsid from handle_events
Changes since v5:
  - Use preallocated MAX_HANDLE_SZ FH buffer
  - Report superblock errors with a zerolength INVALID FID (jan, amir)
---
 fs/notify/fanotify/fanotify.c | 15 +++++++++++++++
 fs/notify/fanotify/fanotify.h |  8 ++++++++
 2 files changed, 23 insertions(+)
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 7032083df62a..8a60c96f5fb2 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -611,7 +611,9 @@ static struct fanotify_event *fanotify_alloc_error_event(
 {
        struct fs_error_report *report =
                        fsnotify_data_error_report(data, data_type);
+       struct inode *inode = report->inode;
        struct fanotify_error_event *fee;
+       int fh_len;

        if (WARN_ON(!report))
                return NULL;
@@ -622,6 +624,19 @@ static struct fanotify_event *fanotify_alloc_error_event(

        fee->fae.type = FANOTIFY_EVENT_TYPE_FS_ERROR;
        fee->err_count = 1;
+       fee->fsid = *fsid;
+
+       fh_len = fanotify_encode_fh_len(inode);
+       if (WARN_ON(fh_len > MAX_HANDLE_SZ)) {
WARN_ON_ONCE please and I rather that this sanity check is moved inside
fanotify_encode_fh_len() where it will return 0 for encoding failure.
quoted hunk ↗ jump to hunk
+               /*
+                * Fallback to reporting the error against the super
+                * block.  It should never happen.
+                */
+               inode = NULL;
+               fh_len = fanotify_encode_fh_len(NULL);
+       }
+
+       fanotify_encode_fh(&fee->object_fh, inode, fh_len, NULL, 0);

        *hash ^= fanotify_hash_fsid(fsid);
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index 2b032b79d5b0..b58400926f92 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -202,6 +202,10 @@ struct fanotify_error_event {
        u32 err_count; /* Suppressed errors count */

        __kernel_fsid_t fsid; /* FSID this error refers to. */
+       /* object_fh must be followed by the inline handle buffer. */
+       struct fanotify_fh object_fh;
+       /* Reserve space in object_fh.buf[] - access with fanotify_fh_buf() */
+       unsigned char _inline_fh_buf[MAX_HANDLE_SZ];
 };
This struct duplicates most of struct fanotify_fid_event.
How about:

#define FANOTIFY_ERROR_FH_LEN \
             (MAX_HANDLE_SZ - FANOTIFY_INLINE_FH_LEN)

struct fanotify_error_event {
         u32 err_count; /* Suppressed errors count */
         struct fanotify_event ffe;
         /* Reserve space in ffe.object_fh.buf[] - access with
fanotify_fh_buf() */
         unsigned char _fh_buf[FANOTIFY_ERROR_FH_LEN];
}

Or leaving out the struct padding and passing
FANOTIFY_ERROR_EVENT_SIZE as mempool object size?

#define FANOTIFY_ERROR_EVENT_SIZE \
            (sizeof(struct fanotify_error_event) + FANOTIFY_ERROR_FH_LEN)

You do not have to make this change - it is a proposal that can have
supporters and objectors, so let's wait to see what you and other reviewers
have to say.

Thanks,
Amir.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help