On Thu, May 16, 2019 at 10:36:32AM +0200, Jan Kara wrote: > On Thu 16-05-19 08:54:37, Amir Goldstein wrote: > > On Wed, May 15, 2019 at 10:33 PM Jan Kara <jack@xxxxxxx> wrote: > > > > > > Proc filesystem has special locking rules for various files. Thus > > > fanotify which opens files on event delivery can easily deadlock > > > against another process that waits for fanotify permission event to be > > > handled. Since permission events on /proc have doubtful value anyway, > > > just disallow them. > > > > > > > Let's add context: > > Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@xxxxxxxxxxxxxx/ > > OK, will add. > > > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > > --- > > > fs/notify/fanotify/fanotify_user.c | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > > > index a90bb19dcfa2..73719949faa6 100644 > > > --- a/fs/notify/fanotify/fanotify_user.c > > > +++ b/fs/notify/fanotify/fanotify_user.c > > > @@ -920,6 +920,20 @@ static int fanotify_test_fid(struct path *path, __kernel_fsid_t *fsid) > > > return 0; > > > } > > > > > > +static int fanotify_events_supported(struct path *path, __u64 mask) > > > +{ > > > + /* > > > + * Proc is special and various files have special locking rules so > > > + * fanotify permission events have high chances of deadlocking the > > > + * system. Just disallow them. > > > + */ > > > + if (mask & FANOTIFY_PERM_EVENTS && > > > + !strcmp(path->mnt->mnt_sb->s_type->name, "proc")) { > > > > Better use an SB_I_ flag to forbid permission events on fs? > > So checking s_type->name indeed felt dirty. I don't think we need a > superblock flag though. I'll probably just go with FS_XXX flag in > file_system_type. Would the same apply for some files that backed by sysfs and reside in /sys? > > > > > + return -EOPNOTSUPP; > > > > I would go with EINVAL following precedent of per filesystem flags > > check on rename(2), but not insisting. > > I was undecided between EOPNOTSUPP and EINVAL. So let's go with EINVAL. I was also thinking that EINVAL makes more sense in this particular case. > > Anyway, following Matthew's man page update for FAN_REPORT_FID, > > we should also add this as reason for EOPNOTSUPP/EINVAL. > > Good point. I've followed up Michael in regards to the FAN_REPORT_FID patch series, but no response as of yet. I'm happy to write the changes for this one if you like? -- Matthew Bobrowski
