| Message ID | 20220329074904.2980320-2-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Evictable fanotify marks | expand |
On Tue 29-03-22 10:48:49, Amir Goldstein wrote: > The inotify mask flags IN_ONESHOT and IN_EXCL_UNLINK are not "internal > to kernel" and should be exposed in procfs fdinfo so CRIU can restore > them. > > Fixes: 6933599697c9 ("inotify: hide internal kernel bits from fdinfo") > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Good catch! > diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c > index 57f0d5d9f934..3451708fd035 100644 > --- a/fs/notify/fdinfo.c > +++ b/fs/notify/fdinfo.c > @@ -83,16 +83,9 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) > inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); > inode = igrab(fsnotify_conn_inode(mark->connector)); > if (inode) { > - /* > - * IN_ALL_EVENTS represents all of the mask bits > - * that we expose to userspace. There is at > - * least one bit (FS_EVENT_ON_CHILD) which is > - * used only internally to the kernel. > - */ > - u32 mask = mark->mask & IN_ALL_EVENTS; > - seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:%x ", > + seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:0 ", > inode_mark->wd, inode->i_ino, inode->i_sb->s_dev, > - mask, mark->ignored_mask); > + inotify_mark_user_mask(mark)); I think inotify_mark_user_mask() helper is overdoing it a bit. Just using INOTIFY_USER_MASK here directly is IMHO fine. Honza
On Tue 05-04-22 14:40:45, Jan Kara wrote: > On Tue 29-03-22 10:48:49, Amir Goldstein wrote: > > diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c > > index 57f0d5d9f934..3451708fd035 100644 > > --- a/fs/notify/fdinfo.c > > +++ b/fs/notify/fdinfo.c > > @@ -83,16 +83,9 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) > > inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); > > inode = igrab(fsnotify_conn_inode(mark->connector)); > > if (inode) { > > - /* > > - * IN_ALL_EVENTS represents all of the mask bits > > - * that we expose to userspace. There is at > > - * least one bit (FS_EVENT_ON_CHILD) which is > > - * used only internally to the kernel. > > - */ > > - u32 mask = mark->mask & IN_ALL_EVENTS; > > - seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:%x ", > > + seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:0 ", > > inode_mark->wd, inode->i_ino, inode->i_sb->s_dev, > > - mask, mark->ignored_mask); > > + inotify_mark_user_mask(mark)); > > I think inotify_mark_user_mask() helper is overdoing it a bit. Just using > INOTIFY_USER_MASK here directly is IMHO fine. Ah, seeing the next patch I take this comment back. Helper is indeed useful. Honza
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c index 57f0d5d9f934..3451708fd035 100644 --- a/fs/notify/fdinfo.c +++ b/fs/notify/fdinfo.c @@ -83,16 +83,9 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); inode = igrab(fsnotify_conn_inode(mark->connector)); if (inode) { - /* - * IN_ALL_EVENTS represents all of the mask bits - * that we expose to userspace. There is at - * least one bit (FS_EVENT_ON_CHILD) which is - * used only internally to the kernel. - */ - u32 mask = mark->mask & IN_ALL_EVENTS; - seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:%x ", + seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:0 ", inode_mark->wd, inode->i_ino, inode->i_sb->s_dev, - mask, mark->ignored_mask); + inotify_mark_user_mask(mark)); show_mark_fhandle(m, inode); seq_putc(m, '\n'); iput(inode); diff --git a/fs/notify/inotify/inotify.h b/fs/notify/inotify/inotify.h index 2007e3711916..8f00151eb731 100644 --- a/fs/notify/inotify/inotify.h +++ b/fs/notify/inotify/inotify.h @@ -22,6 +22,18 @@ static inline struct inotify_event_info *INOTIFY_E(struct fsnotify_event *fse) return container_of(fse, struct inotify_event_info, fse); } +/* + * INOTIFY_USER_FLAGS represents all of the mask bits that we expose to + * userspace. There is at least one bit (FS_EVENT_ON_CHILD) which is + * used only internally to the kernel. + */ +#define INOTIFY_USER_MASK (IN_ALL_EVENTS | IN_ONESHOT | IN_EXCL_UNLINK) + +static inline __u32 inotify_mark_user_mask(struct fsnotify_mark *fsn_mark) +{ + return fsn_mark->mask & INOTIFY_USER_MASK; +} + extern void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark, struct fsnotify_group *group); extern int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 54583f62dc44..3ef57db0ec9d 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -110,7 +110,7 @@ static inline __u32 inotify_arg_to_mask(struct inode *inode, u32 arg) mask |= FS_EVENT_ON_CHILD; /* mask off the flags used to open the fd */ - mask |= (arg & (IN_ALL_EVENTS | IN_ONESHOT | IN_EXCL_UNLINK)); + mask |= (arg & INOTIFY_USER_MASK); return mask; }
The inotify mask flags IN_ONESHOT and IN_EXCL_UNLINK are not "internal to kernel" and should be exposed in procfs fdinfo so CRIU can restore them. Fixes: 6933599697c9 ("inotify: hide internal kernel bits from fdinfo") Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/notify/fdinfo.c | 11 ++--------- fs/notify/inotify/inotify.h | 12 ++++++++++++ fs/notify/inotify/inotify_user.c | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-)