2021-10-25 20:10:25

by Gabriel Krisman Bertazi

Subject: [PATCH v9 11/31] fsnotify: Protect fsnotify_handle_inode_event from no-inode events

FAN_FS_ERROR allows events without inodes - i.e. for file system-wide
errors. Even though fsnotify_handle_inode_event is not currently used
by fanotify, this patch protects other backends from cases where neither
inode or dir are provided. Also document the constraints of the
interface (inode and dir cannot be both NULL).

Suggested-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

---
Changes since v8:
- Convert verifications to WARN_ON
- Require either inode or dir
- Protect nfsd backend from !inode.
---
fs/nfsd/filecache.c | 3 +++
fs/notify/fsnotify.c | 3 +++
include/linux/fsnotify_backend.h | 1 +
3 files changed, 7 insertions(+)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index be3c1aad50ea..fdf89fcf1a0c 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -602,6 +602,9 @@ nfsd_file_fsnotify_handle_event(struct fsnotify_mark *mark, u32 mask,
struct inode *inode, struct inode *dir,
const struct qstr *name, u32 cookie)
{
+ if (WARN_ON_ONCE(!inode))
+ return 0;
+
trace_nfsd_file_fsnotify_handle_event(inode, mask);

/* Should be no marks on non-regular files */
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index fde3a1115a17..4034ca566f95 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -252,6 +252,9 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
if (WARN_ON_ONCE(!ops->handle_inode_event))
return 0;

+ if (WARN_ON_ONCE(!inode && !dir))
+ return 0;
+
if ((inode_mark->mask & FS_EXCL_UNLINK) &&
path && d_unlinked(path->dentry))
return 0;
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 035438fe4a43..b71dc788018e 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -136,6 +136,7 @@ struct mem_cgroup;
* @dir: optional directory associated with event -
* if @file_name is not NULL, this is the directory that
* @file_name is relative to.
+ * Either @inode or @dir must be non-NULL.
* @file_name: optional file name associated with event
* @cookie: inotify rename cookie
*
--
2.33.0

2021-10-26 07:22:01

by Amir Goldstein

Subject: Re: [PATCH v9 11/31] fsnotify: Protect fsnotify_handle_inode_event from no-inode events

On Mon, Oct 25, 2021 at 10:29 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> FAN_FS_ERROR allows events without inodes - i.e. for file system-wide
> errors. Even though fsnotify_handle_inode_event is not currently used
> by fanotify, this patch protects other backends from cases where neither
> inode or dir are provided. Also document the constraints of the
> interface (inode and dir cannot be both NULL).
>
> Suggested-by: Amir Goldstein <[email protected]>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>

Reviewed-by: Amir Goldstein <[email protected]>

> ---
> Changes since v8:
> - Convert verifications to WARN_ON
> - Require either inode or dir
> - Protect nfsd backend from !inode.
> ---
> fs/nfsd/filecache.c | 3 +++
> fs/notify/fsnotify.c | 3 +++
> include/linux/fsnotify_backend.h | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index be3c1aad50ea..fdf89fcf1a0c 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -602,6 +602,9 @@ nfsd_file_fsnotify_handle_event(struct fsnotify_mark *mark, u32 mask,
> struct inode *inode, struct inode *dir,
> const struct qstr *name, u32 cookie)
> {
> + if (WARN_ON_ONCE(!inode))
> + return 0;
> +
> trace_nfsd_file_fsnotify_handle_event(inode, mask);
>
> /* Should be no marks on non-regular files */
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index fde3a1115a17..4034ca566f95 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -252,6 +252,9 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
> if (WARN_ON_ONCE(!ops->handle_inode_event))
> return 0;
>
> + if (WARN_ON_ONCE(!inode && !dir))
> + return 0;
> +
> if ((inode_mark->mask & FS_EXCL_UNLINK) &&
> path && d_unlinked(path->dentry))
> return 0;
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 035438fe4a43..b71dc788018e 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -136,6 +136,7 @@ struct mem_cgroup;
> * @dir: optional directory associated with event -
> * if @file_name is not NULL, this is the directory that
> * @file_name is relative to.
> + * Either @inode or @dir must be non-NULL.
> * @file_name: optional file name associated with event
> * @cookie: inotify rename cookie
> *
> --
> 2.33.0
>

2021-10-26 14:26:11

by Jan Kara

Subject: Re: [PATCH v9 11/31] fsnotify: Protect fsnotify_handle_inode_event from no-inode events

On Mon 25-10-21 16:27:26, Gabriel Krisman Bertazi wrote:
> FAN_FS_ERROR allows events without inodes - i.e. for file system-wide
> errors. Even though fsnotify_handle_inode_event is not currently used
> by fanotify, this patch protects other backends from cases where neither
> inode or dir are provided. Also document the constraints of the
> interface (inode and dir cannot be both NULL).
>
> Suggested-by: Amir Goldstein <[email protected]>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

>
> ---
> Changes since v8:
> - Convert verifications to WARN_ON
> - Require either inode or dir
> - Protect nfsd backend from !inode.
> ---
> fs/nfsd/filecache.c | 3 +++
> fs/notify/fsnotify.c | 3 +++
> include/linux/fsnotify_backend.h | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index be3c1aad50ea..fdf89fcf1a0c 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -602,6 +602,9 @@ nfsd_file_fsnotify_handle_event(struct fsnotify_mark *mark, u32 mask,
> struct inode *inode, struct inode *dir,
> const struct qstr *name, u32 cookie)
> {
> + if (WARN_ON_ONCE(!inode))
> + return 0;
> +
> trace_nfsd_file_fsnotify_handle_event(inode, mask);
>
> /* Should be no marks on non-regular files */
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index fde3a1115a17..4034ca566f95 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -252,6 +252,9 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
> if (WARN_ON_ONCE(!ops->handle_inode_event))
> return 0;
>
> + if (WARN_ON_ONCE(!inode && !dir))
> + return 0;
> +
> if ((inode_mark->mask & FS_EXCL_UNLINK) &&
> path && d_unlinked(path->dentry))
> return 0;
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 035438fe4a43..b71dc788018e 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -136,6 +136,7 @@ struct mem_cgroup;
> * @dir: optional directory associated with event -
> * if @file_name is not NULL, this is the directory that
> * @file_name is relative to.
> + * Either @inode or @dir must be non-NULL.
> * @file_name: optional file name associated with event
> * @cookie: inotify rename cookie
> *
> --
> 2.33.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR