On Thu 25-07-24 14:19:45, Josef Bacik wrote:
> From: Amir Goldstein <amir73il@xxxxxxxxx>
>
> With group class FAN_CLASS_PRE_CONTENT, report offset and length info
> along with FAN_PRE_ACCESS and FAN_PRE_MODIFY permission events.
>
> This information is meant to be used by hierarchical storage managers
> that want to fill partial content of files on first access to range.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.h | 8 +++++++
> fs/notify/fanotify/fanotify_user.c | 38 ++++++++++++++++++++++++++++++
> include/uapi/linux/fanotify.h | 7 ++++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 93598b7d5952..7f06355afa1f 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -448,6 +448,14 @@ static inline bool fanotify_is_perm_event(u32 mask)
> mask & FANOTIFY_PERM_EVENTS;
> }
>
> +static inline bool fanotify_event_has_access_range(struct fanotify_event *event)
> +{
> + if (!(event->mask & FANOTIFY_PRE_CONTENT_EVENTS))
> + return false;
> +
> + return FANOTIFY_PERM(event)->ppos;
> +}
Now I'm a bit confused. Can we have legally NULL ppos for an event from
FANOTIFY_PRE_CONTENT_EVENTS?
> +static size_t copy_range_info_to_user(struct fanotify_event *event,
> + char __user *buf, int count)
> +{
> + struct fanotify_perm_event *pevent = FANOTIFY_PERM(event);
> + struct fanotify_event_info_range info = { };
> + size_t info_len = FANOTIFY_RANGE_INFO_LEN;
> +
> + if (WARN_ON_ONCE(info_len > count))
> + return -EFAULT;
> +
> + if (WARN_ON_ONCE(!pevent->ppos))
> + return 0;
I think we should be returning some error here. Maybe EINVAL? Otherwise
fanotify_event_len() will return different length than we actually generate
and that could lead to strange failures later.
> +
> + info.hdr.info_type = FAN_EVENT_INFO_TYPE_RANGE;
> + info.hdr.len = info_len;
> + info.offset = *(pevent->ppos);
> + info.count = pevent->count;
> +
> + if (copy_to_user(buf, &info, info_len))
> + return -EFAULT;
> +
> + return info_len;
> +}
> +
> static int copy_info_records_to_user(struct fanotify_event *event,
> struct fanotify_info *info,
> unsigned int info_mode, int pidfd,
...
> @@ -191,6 +192,12 @@ struct fanotify_event_info_error {
> __u32 error_count;
> };
>
> +struct fanotify_event_info_range {
> + struct fanotify_event_info_header hdr;
> + __u32 count;
> + __u64 offset;
> +};
Just for the sake of future-proofing, I'd make 'count' u64. I understand it
means growing fanotify_event_info_range by 8 bytes and we currently never
do reads or writes larger than 2 GB but I don't think saving bytes like
this is really a good tradeoff these days...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
