* Re: [PATCH v4 0/4] fanotify: Allow user space to pass back additional audit info
[not found] <cover.1659996830.git.rgb@redhat.com>
@ 2022-08-10 5:21 ` Amir Goldstein
[not found] ` <8767f3a0d43d6a994584b86c03eb659a662cc416.1659996830.git.rgb@redhat.com>
1 sibling, 0 replies; 3+ messages in thread
From: Amir Goldstein @ 2022-08-10 5:21 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Linux-Audit Mailing List, LKML, linux-fsdevel, Paul Moore,
Eric Paris, Steve Grubb, Jan Kara, Linux API
[+linux-api]
On Tue, Aug 9, 2022 at 7:23 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> The Fanotify API can be used for access control by requesting permission
> event notification. The user space tooling that uses it may have a
> complicated policy that inherently contains additional context for the
> decision. If this information were available in the audit trail, policy
> writers can close the loop on debugging policy. Also, if this additional
> information were available, it would enable the creation of tools that
> can suggest changes to the policy similar to how audit2allow can help
> refine labeled security.
>
> This patchset defines a new flag (FAN_INFO) and new extensions that
> define additional information which are appended after the response
> structure returned from user space on a permission event. The appended
> information is organized with headers containing a type and size that
> can be delegated to interested subsystems. One new information type is
> defined for audit rule number.
>
> A newer kernel will work with an older userspace and an older kernel
> will behave as expected and reject a newer userspace, leaving it up to
> the newer userspace to test appropriately and adapt as necessary.
Since you did not accept my suggestion of FAN_TEST response code [1],
I am not really sure how you envision that "adapt as necessary" part.
A well designed UAPI should allow newer userspace to check for kernel
support of FAN_INFO on initialization.
For example, without this property of UAPI, it is going to be hard to write an
LTP test for the new functionality that does not run on older kernels.
As far as I can tell, your proposed UAPI does not provide this functionality.
It allows newer userspace to check for support of FAN_INFO only as a
response to a permission event.
You never replied to my question to explain why you think FAN_TEST
complicated things. The only purpose of FAN_TEST is for userspace
to be able to test FAN_INFO kernel support without providing a valid fd.
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxi+8HUqyGxQBNMqSong92nreOWLKdy9MCrYg8wgW9Dj4g@mail.gmail.com/
>
> The audit function was updated to log the additional information in the
> AUDIT_FANOTIFY record. The following is an example of the new record
> format:
>
> type=FANOTIFY msg=audit(1600385147.372:590): resp=2 fan_type=1 fan_info=3F
>
> changelog:
> v1:
> - first version by Steve Grubb <sgrubb@redhat.com>
> Link: https://lore.kernel.org/r/2042449.irdbgypaU6@x2
>
> v2:
> - enhancements suggested by Jan Kara <jack@suse.cz>
> - 1/3 change %d to %u in pr_debug
> - 2/3 change response from __u32 to __u16
> - mod struct fanotify_response and fanotify_perm_event add extra_info_type, extra_info_buf
> - extra_info_buf size max FANOTIFY_MAX_RESPONSE_EXTRA_LEN, add struct fanotify_response_audit_rule
> - extend debug statements
> - remove unneeded macros
> - [internal] change interface to finish_permission_event() and process_access_response()
> - 3/3 update format of extra information
> - [internal] change interface to audit_fanotify()
> - change ctx_type= to fan_type=
> Link: https://lore.kernel.org/r/cover.1651174324.git.rgb@redhat.com
>
> v3:
> - 1/3 switch {,__}audit_fanotify() from uint to u32
> - 2/3 re-add fanotify_get_response switch case FAN_DENY: to avoid unnecessary churn
> - add FAN_EXTRA flag to indicate more info and break with old kernel
> - change response from u16 to u32 to avoid endian issues
> - change extra_info_buf to union
> - move low-cost fd check earlier
> - change FAN_RESPONSE_INFO_AUDIT_NONE to FAN_RESPONSE_INFO_NONE
> - switch to u32 for internal and __u32 for uapi
> Link: https://lore.kernel.org/r/cover.1652724390.git.rgb@redhat.com
Link seems broken?
>
> v4:
> - scrap FAN_INVALID_RESPONSE_MASK in favour of original to catch invalid response == 0
> - introduce FANOTIFY_RESPONSE_* macros
> - uapi: remove union
> - keep original struct fanotify_response, add fan_info infra starting with audit reason
> - uapi add struct fanotify_response_info_header{type/pad/len} and struct fanotify_response_info_audit_rule{hdr/rule}
> - rename fan_ctx= to fan_info=, FAN_EXTRA to FAN_INFO
> - change event struct from type/buf to len/buf
> - enable multiple info extensions in one message
> - hex encode fan_info in __audit_fanotify()
> - record type FANOTIFY extended to "type=FANOTIFY msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=3F"
> Link: https://lore.kernel.org/r/cover.1659981772.git.rgb@redhat.com
Link seems broken?
Thanks,
Amir.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
[not found] ` <8767f3a0d43d6a994584b86c03eb659a662cc416.1659996830.git.rgb@redhat.com>
@ 2022-08-10 6:22 ` Amir Goldstein
2022-08-19 11:24 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2022-08-10 6:22 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Linux-Audit Mailing List, LKML, linux-fsdevel, Paul Moore,
Eric Paris, Steve Grubb, Jan Kara, Linux API
[+linux-api]
On Tue, Aug 9, 2022 at 7:23 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> This patch adds a flag, FAN_INFO and an extensible buffer to provide
> additional information about response decisions. The buffer contains
> one or more headers defining the information type and the length of the
> following information. The patch defines one additional information
> type, FAN_RESPONSE_INFO_AUDIT_RULE, an audit rule number. This will
> allow for the creation of other information types in the future if other
> users of the API identify different needs.
>
> Suggested-by: Steve Grubb <sgrubb@redhat.com>
> Link: https://lore.kernel.org/r/2745105.e9J7NaK4W3@x2
> Suggested-by: Jan Kara <jack@suse.cz>
> Link: https://lore.kernel.org/r/20201001101219.GE17860@quack2.suse.cz
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
Looks mostly fine.
A few small bugs and style suggestions
and one UAPI improvement suggestion.
> fs/notify/fanotify/fanotify.c | 10 ++-
> fs/notify/fanotify/fanotify.h | 2 +
> fs/notify/fanotify/fanotify_user.c | 104 +++++++++++++++++++++++------
> include/linux/fanotify.h | 5 ++
> include/uapi/linux/fanotify.h | 27 +++++++-
> 5 files changed, 123 insertions(+), 25 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 4f897e109547..0f36062521f4 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -262,13 +262,16 @@ static int fanotify_get_response(struct fsnotify_group *group,
> }
>
> /* userspace responded, convert to something usable */
> - switch (event->response & ~FAN_AUDIT) {
> + switch (event->response & FANOTIFY_RESPONSE_ACCESS) {
> case FAN_ALLOW:
> ret = 0;
> break;
> case FAN_DENY:
> - default:
> ret = -EPERM;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
This is very odd.
Why has this changed?
The return value here is going to the process that
is trying to access the file.
> }
>
> /* Check if the response should be audited */
> @@ -560,6 +563,8 @@ static struct fanotify_event *fanotify_alloc_perm_event(const struct path *path,
>
> pevent->fae.type = FANOTIFY_EVENT_TYPE_PATH_PERM;
> pevent->response = 0;
> + pevent->info_len = 0;
> + pevent->info_buf = NULL;
> pevent->state = FAN_EVENT_INIT;
> pevent->path = *path;
> path_get(path);
> @@ -996,6 +1001,7 @@ static void fanotify_free_path_event(struct fanotify_event *event)
> static void fanotify_free_perm_event(struct fanotify_event *event)
> {
> path_put(fanotify_event_path(event));
> + kfree(FANOTIFY_PERM(event)->info_buf);
> kmem_cache_free(fanotify_perm_event_cachep, FANOTIFY_PERM(event));
> }
>
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index abfa3712c185..14c30e173632 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -428,6 +428,8 @@ struct fanotify_perm_event {
> u32 response; /* userspace answer to the event */
> unsigned short state; /* state of the event */
> int fd; /* fd we passed to userspace for this event */
> + size_t info_len;
> + char *info_buf;
> };
>
> static inline struct fanotify_perm_event *
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index ff67ca0d25cc..a4ae953f0e62 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -289,13 +289,18 @@ static int create_fd(struct fsnotify_group *group, struct path *path,
> */
> static void finish_permission_event(struct fsnotify_group *group,
> struct fanotify_perm_event *event,
> - u32 response)
> + struct fanotify_response *response,
> + size_t info_len, char *info_buf)
> __releases(&group->notification_lock)
> {
> bool destroy = false;
>
> assert_spin_locked(&group->notification_lock);
> - event->response = response;
> + event->response = response->response & ~FAN_INFO;
> + if (response->response & FAN_INFO) {
> + event->info_len = info_len;
> + event->info_buf = info_buf;
> + }
> if (event->state == FAN_EVENT_CANCELED)
> destroy = true;
> else
> @@ -306,33 +311,71 @@ static void finish_permission_event(struct fsnotify_group *group,
> }
>
> static int process_access_response(struct fsnotify_group *group,
> - struct fanotify_response *response_struct)
> + struct fanotify_response *response_struct,
> + const char __user *buf,
> + size_t count)
> {
> struct fanotify_perm_event *event;
> int fd = response_struct->fd;
> u32 response = response_struct->response;
> + struct fanotify_response_info_header info_hdr;
> + char *info_buf = NULL;
>
> - pr_debug("%s: group=%p fd=%d response=%u\n", __func__, group,
> - fd, response);
> + pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
> + group, fd, response, info_buf, count);
> /*
> * make sure the response is valid, if invalid we do nothing and either
> * userspace can send a valid response or we will clean it up after the
> * timeout
> */
> - switch (response & ~FAN_AUDIT) {
> + if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
> + return -EINVAL;
> + switch (response & FANOTIFY_RESPONSE_ACCESS) {
> case FAN_ALLOW:
> case FAN_DENY:
> break;
> default:
> return -EINVAL;
> }
> -
> - if (fd < 0)
> - return -EINVAL;
> -
> if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
> return -EINVAL;
> + if (fd < 0)
> + return -EINVAL;
Since you did not accept my suggestion of FAN_TEST [1],
I am not sure why this check was moved.
However, if you move this check past FAN_INFO processing,
you could change the error value to -ENOENT, same as the return value
for an fd that is >= 0 but does not correspond to any pending
permission event.
The idea was that userspace could write a test
fanotify_response_info_audit_rule payload to fanotify fd with FAN_NOFD
in the response.fd field.
On old kernel, this will return EINVAL.
On new kernel, if the fanotify_response_info_audit_rule payload
passes all the validations, this will do nothing and return ENOENT.
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxi+8HUqyGxQBNMqSong92nreOWLKdy9MCrYg8wgW9Dj4g@mail.gmail.com/
> + if (response & FAN_INFO) {
Please split this out to helper process_response_info() and
optionally also helper process_response_info_audit_rule()
> + size_t c = count;
> + const char __user *ib = buf;
>
> + if (c <= 0)
> + return -EINVAL;
This was already checked by the caller.
If you think we need this defence use if (WARN_ON_ONCE())
> + while (c >= sizeof(info_hdr)) {
This while() is a bit confusing.
It suggests that the parser may process multiple info records,
but the code below uses 'count' and assumed single audit rule
record.
Maybe just change this to:
if (WARN_ON_ONCE(c < sizeof(info_hdr))
return -EINVAL
Until the code can really handle multiple records.
> + if (copy_from_user(&info_hdr, ib, sizeof(info_hdr)))
> + return -EFAULT;
> + if (info_hdr.pad != 0)
> + return -EINVAL;
> + if (c < info_hdr.len)
> + return -EINVAL;
> + switch (info_hdr.type) {
> + case FAN_RESPONSE_INFO_AUDIT_RULE:
> + break;
> + case FAN_RESPONSE_INFO_NONE:
> + default:
> + return -EINVAL;
> + }
> + c -= info_hdr.len;
> + ib += info_hdr.len;
> + }
> + if (c != 0)
> + return -EINVAL;
> + /* Simplistic check for now */
> + if (count != sizeof(struct fanotify_response_info_audit_rule))
> + return -EINVAL;
> + info_buf = kmalloc(sizeof(struct fanotify_response_info_audit_rule),
> + GFP_KERNEL);
> + if (!info_buf)
> + return -ENOMEM;
> + if (copy_from_user(info_buf, buf, count))
> + return -EFAULT;
info_buf allocation is leaked here and also in case 'fd' is not found.
> + }
> spin_lock(&group->notification_lock);
> list_for_each_entry(event, &group->fanotify_data.access_list,
> fae.fse.list) {
> @@ -340,7 +383,9 @@ static int process_access_response(struct fsnotify_group *group,
> continue;
>
> list_del_init(&event->fae.fse.list);
> - finish_permission_event(group, event, response);
> + /* finish_permission_event() eats info_buf */
> + finish_permission_event(group, event, response_struct,
> + count, info_buf);
> wake_up(&group->fanotify_data.access_waitq);
> return 0;
> }
> @@ -802,9 +847,14 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
> fsnotify_destroy_event(group, &event->fse);
> } else {
> if (ret <= 0) {
> + struct fanotify_response response = {
> + .fd = FAN_NOFD,
> + .response = FAN_DENY };
> +
> spin_lock(&group->notification_lock);
> finish_permission_event(group,
> - FANOTIFY_PERM(event), FAN_DENY);
> + FANOTIFY_PERM(event), &response,
> + 0, NULL);
> wake_up(&group->fanotify_data.access_waitq);
> } else {
> spin_lock(&group->notification_lock);
> @@ -827,26 +877,33 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
>
> static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
> {
> - struct fanotify_response response = { .fd = -1, .response = -1 };
> + struct fanotify_response response;
> struct fsnotify_group *group;
> int ret;
> + const char __user *info_buf = buf + sizeof(struct fanotify_response);
> + size_t c;
>
> if (!IS_ENABLED(CONFIG_FANOTIFY_ACCESS_PERMISSIONS))
> return -EINVAL;
>
> group = file->private_data;
>
> - if (count < sizeof(response))
> - return -EINVAL;
> -
> - count = sizeof(response);
> -
> pr_debug("%s: group=%p count=%zu\n", __func__, group, count);
>
> - if (copy_from_user(&response, buf, count))
> + if (count < sizeof(response))
> + return -EINVAL;
> + if (copy_from_user(&response, buf, sizeof(response)))
> return -EFAULT;
>
> - ret = process_access_response(group, &response);
> + c = count - sizeof(response);
> + if (response.response & FAN_INFO) {
> + if (c < sizeof(struct fanotify_response_info_header))
> + return -EINVAL;
Should FAN_INFO require FAN_AUDIT?
> + } else {
> + if (c != 0)
> + return -EINVAL;
> + }
> + ret = process_access_response(group, &response, info_buf, c);
> if (ret < 0)
> count = ret;
>
> @@ -857,6 +914,9 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> {
> struct fsnotify_group *group = file->private_data;
> struct fsnotify_event *fsn_event;
> + struct fanotify_response response = {
> + .fd = FAN_NOFD,
> + .response = FAN_ALLOW };
>
> /*
> * Stop new events from arriving in the notification queue. since
> @@ -876,7 +936,7 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> event = list_first_entry(&group->fanotify_data.access_list,
> struct fanotify_perm_event, fae.fse.list);
> list_del_init(&event->fae.fse.list);
> - finish_permission_event(group, event, FAN_ALLOW);
> + finish_permission_event(group, event, &response, 0, NULL);
> spin_lock(&group->notification_lock);
> }
>
> @@ -893,7 +953,7 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> fsnotify_destroy_event(group, fsn_event);
> } else {
> finish_permission_event(group, FANOTIFY_PERM(event),
> - FAN_ALLOW);
> + &response, 0, NULL);
> }
> spin_lock(&group->notification_lock);
> }
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index edc28555814c..ce9f97eb69f2 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -114,6 +114,11 @@
> #define ALL_FANOTIFY_EVENT_BITS (FANOTIFY_OUTGOING_EVENTS | \
> FANOTIFY_EVENT_FLAGS)
>
> +/* This mask is to check for invalid bits of a user space permission response */
> +#define FANOTIFY_RESPONSE_ACCESS (FAN_ALLOW | FAN_DENY)
> +#define FANOTIFY_RESPONSE_FLAGS (FAN_AUDIT | FAN_INFO)
> +#define FANOTIFY_RESPONSE_VALID_MASK (FANOTIFY_RESPONSE_ACCESS | FANOTIFY_RESPONSE_FLAGS)
> +
> /* Do not use these old uapi constants internally */
> #undef FAN_ALL_CLASS_BITS
> #undef FAN_ALL_INIT_FLAGS
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index f1f89132d60e..4d08823a5698 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -180,15 +180,40 @@ struct fanotify_event_info_error {
> __u32 error_count;
> };
>
> +/*
> + * User space may need to record additional information about its decision.
> + * The extra information type records what kind of information is included.
> + * The default is none. We also define an extra information buffer whose
> + * size is determined by the extra information type.
> + *
> + * If the context type is Rule, then the context following is the rule number
> + * that triggered the user space decision.
> + */
> +
> +#define FAN_RESPONSE_INFO_NONE 0
> +#define FAN_RESPONSE_INFO_AUDIT_RULE 1
> +
> struct fanotify_response {
> __s32 fd;
> __u32 response;
> };
>
> +struct fanotify_response_info_header {
> + __u8 type;
> + __u8 pad;
> + __u16 len;
> +};
> +
> +struct fanotify_response_info_audit_rule {
> + struct fanotify_response_info_header hdr;
> + __u32 audit_rule;
> +};
> +
> /* Legit userspace responses to a _PERM event */
> #define FAN_ALLOW 0x01
> #define FAN_DENY 0x02
> -#define FAN_AUDIT 0x10 /* Bit mask to create audit record for result */
> +#define FAN_AUDIT 0x10 /* Bitmask to create audit record for result */
> +#define FAN_INFO 0x20 /* Bitmask to indicate additional information */
>
> /* No fd set in event */
> #define FAN_NOFD -1
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
2022-08-10 6:22 ` [PATCH v4 2/4] fanotify: define struct members to hold response decision context Amir Goldstein
@ 2022-08-19 11:24 ` Jan Kara
0 siblings, 0 replies; 3+ messages in thread
From: Jan Kara @ 2022-08-19 11:24 UTC (permalink / raw)
To: Amir Goldstein
Cc: Richard Guy Briggs, Linux-Audit Mailing List, LKML,
linux-fsdevel, Paul Moore, Eric Paris, Steve Grubb, Jan Kara,
Linux API
On Wed 10-08-22 08:22:49, Amir Goldstein wrote:
> [+linux-api]
>
> On Tue, Aug 9, 2022 at 7:23 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > This patch adds a flag, FAN_INFO and an extensible buffer to provide
> > additional information about response decisions. The buffer contains
> > one or more headers defining the information type and the length of the
> > following information. The patch defines one additional information
> > type, FAN_RESPONSE_INFO_AUDIT_RULE, an audit rule number. This will
> > allow for the creation of other information types in the future if other
> > users of the API identify different needs.
> >
> > Suggested-by: Steve Grubb <sgrubb@redhat.com>
> > Link: https://lore.kernel.org/r/2745105.e9J7NaK4W3@x2
> > Suggested-by: Jan Kara <jack@suse.cz>
> > Link: https://lore.kernel.org/r/20201001101219.GE17860@quack2.suse.cz
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
...
> > static int process_access_response(struct fsnotify_group *group,
> > - struct fanotify_response *response_struct)
> > + struct fanotify_response *response_struct,
> > + const char __user *buf,
> > + size_t count)
> > {
> > struct fanotify_perm_event *event;
> > int fd = response_struct->fd;
> > u32 response = response_struct->response;
> > + struct fanotify_response_info_header info_hdr;
> > + char *info_buf = NULL;
> >
> > - pr_debug("%s: group=%p fd=%d response=%u\n", __func__, group,
> > - fd, response);
> > + pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
> > + group, fd, response, info_buf, count);
> > /*
> > * make sure the response is valid, if invalid we do nothing and either
> > * userspace can send a valid response or we will clean it up after the
> > * timeout
> > */
> > - switch (response & ~FAN_AUDIT) {
> > + if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
> > + return -EINVAL;
> > + switch (response & FANOTIFY_RESPONSE_ACCESS) {
> > case FAN_ALLOW:
> > case FAN_DENY:
> > break;
> > default:
> > return -EINVAL;
> > }
> > -
> > - if (fd < 0)
> > - return -EINVAL;
> > -
> > if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
> > return -EINVAL;
> > + if (fd < 0)
> > + return -EINVAL;
>
> Since you did not accept my suggestion of FAN_TEST [1],
> I am not sure why this check was moved.
>
> However, if you move this check past FAN_INFO processing,
> you could change the error value to -ENOENT, same as the return value
> for an fd that is >= 0 but does not correspond to any pending
> permission event.
>
> The idea was that userspace could write a test
> fanotify_response_info_audit_rule payload to fanotify fd with FAN_NOFD
> in the response.fd field.
> On old kernel, this will return EINVAL.
> On new kernel, if the fanotify_response_info_audit_rule payload
> passes all the validations, this will do nothing and return ENOENT.
>
> [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxi+8HUqyGxQBNMqSong92nreOWLKdy9MCrYg8wgW9Dj4g@mail.gmail.com/
Yes. Richard, if you don't like the FAN_TEST proposal from Amir, please
explain (preferably also with sample code) how you imagine userspace will
decide whether to use FAN_INFO flag in responses or not. Because if it will
just blindly set it, that will result in all permission events to finished
with EPERM for kernels not recognizing FAN_INFO.
> > - if (count < sizeof(response))
> > - return -EINVAL;
> > -
> > - count = sizeof(response);
> > -
> > pr_debug("%s: group=%p count=%zu\n", __func__, group, count);
> >
> > - if (copy_from_user(&response, buf, count))
> > + if (count < sizeof(response))
> > + return -EINVAL;
> > + if (copy_from_user(&response, buf, sizeof(response)))
> > return -EFAULT;
> >
> > - ret = process_access_response(group, &response);
> > + c = count - sizeof(response);
> > + if (response.response & FAN_INFO) {
> > + if (c < sizeof(struct fanotify_response_info_header))
> > + return -EINVAL;
>
> Should FAN_INFO require FAN_AUDIT?
Currently we could but longer term not all additional info needs to be
related to audit so probably I'd not require that even now (which results
in info being effectively ignored after it is parsed).
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-19 11:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1659996830.git.rgb@redhat.com>
2022-08-10 5:21 ` [PATCH v4 0/4] fanotify: Allow user space to pass back additional audit info Amir Goldstein
[not found] ` <8767f3a0d43d6a994584b86c03eb659a662cc416.1659996830.git.rgb@redhat.com>
2022-08-10 6:22 ` [PATCH v4 2/4] fanotify: define struct members to hold response decision context Amir Goldstein
2022-08-19 11:24 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).