On Sun 02-12-18 13:38:20, Amir Goldstein wrote:
> @@ -268,6 +289,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> int ret = 0;
> struct fanotify_event *event;
> struct fsnotify_event *fsn_event;
> + __kernel_fsid_t __fsid, *fsid = NULL;
>
> BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
> BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
> @@ -300,7 +322,10 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> return 0;
> }
>
> - event = fanotify_alloc_event(group, inode, mask, data);
> + if (FAN_GROUP_FLAG(group, FAN_REPORT_FID))
> + fsid = fanotify_get_fsid(iter_info, &__fsid);
> +
> + event = fanotify_alloc_event(group, inode, mask, data, fsid);
This looks wrong? fsid is never assigned anything != NULL? How could have
this worked?
> @@ -853,9 +855,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> }
>
> /* Check if filesystem can encode a unique fid */
> -static int fanotify_test_fid(struct path *path)
> +static int fanotify_test_fid(struct path *path, struct kstatfs *stat)
> {
> - struct kstatfs stat, root_stat;
> + struct kstatfs root_stat;
Any reason why you don't just return fsid from fanotify_test_fid() instead
of full kstatfs structure? The extra copy of 8 bytes does not really hurt
us...
> @@ -553,15 +560,33 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
>
> if (WARN_ON(!fsnotify_valid_obj_type(type)))
> return -EINVAL;
> +
> + /* Backend is expected to check for zero fsid (e.g. tmpfs) */
> + if (fsid && WARN_ON_ONCE(!fsid->val[0] && !fsid->val[1]))
> + return -ENODEV;
> +
> restart:
> spin_lock(&mark->lock);
> conn = fsnotify_grab_connector(connp);
> if (!conn) {
> spin_unlock(&mark->lock);
> - err = fsnotify_attach_connector_to_object(connp, type);
> + err = fsnotify_attach_connector_to_object(connp, type, fsid);
> if (err)
> return err;
> goto restart;
> + } else if (fsid &&
> + (fsid->val[0] != conn->fsid.val[0] ||
> + fsid->val[1] != conn->fsid.val[1])) {
I think you need to allow transition from 0 to something != 0 here.
Otherwise you'll get this warning if someone attaches say inotify mark to
an inode and then fanotify mark for FAN_REPORT_FID group.
> + /*
> + * Backend is expected to check for non uniform fsid
> + * (e.g. btrfs), but maybe we missed something?
> + */
> + pr_warn_ratelimited("%s: fsid mismatch on object of type %u: %x.%x != %x.%x\n",
> + __func__, conn->type,
> + fsid->val[0], fsid->val[1],
> + conn->fsid.val[0], conn->fsid.val[1]);
> + err = -EXDEV;
> + goto out_err;
> }
>
> /* is mark the first mark? */
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
