Re: [PATCH v5 3/3] fanotify,audit: Allow audit to use the full permission event response

Hello, Sorry to take so long. Holidays and kernel build problems. However, I have built a kernel with these patches. I only have 2 comments. When I use an application that expected the old API, meaning it simply does: response.fd = metadata->fd; response.response = reply; close(metadata->fd); write(fd, &response, sizeof(struct fanotify_response)); I get access denials. Every time. If the program is using the new API and sets FAN_INFO, then it works as expected. I'll do some more testing but I think there is something wrong in the compatibility path.

On Monday, December 12, 2022 9:06:11 AM EST Richard Guy Briggs wrote: > This patch passes the full response so that the audit function can use all > of it. The audit function was updated to log the additional information in > the AUDIT_FANOTIFY record. What I'm seeing is: type=FANOTIFY msg=audit(01/09/2023 18:43:16.306:366) : resp=deny fan_type=1 fan_info=313300000000000000000000 subj_trust=0 obj_trust=0 Where fan_info was supposed to be 13 decimal. More below...

> Currently the only type of fanotify info that is defined is an audit > rule number, but convert it to hex encoding to future-proof the field. > Hex encoding suggested by Paul Moore <paul(a)paul-moore.com&gt;. > > Sample records: > type=FANOTIFY msg=audit(1600385147.372:590): resp=2 fan_type=1 > fan_info=3137 subj_trust=3 obj_trust=5 type=FANOTIFY > msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=3F subj_trust=2 > obj_trust=2 > > Suggested-by: Steve Grubb <sgrubb(a)redhat.com&gt; > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > --- > fs/notify/fanotify/fanotify.c | 3 ++- > include/linux/audit.h | 9 +++++---- > kernel/auditsc.c | 25 ++++++++++++++++++++++--- > 3 files changed, 29 insertions(+), 8 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index 24ec1d66d5a8..29bdd99b29fa 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -273,7 +273,8 @@ static int fanotify_get_response(struct fsnotify_group > *group, > > /* Check if the response should be audited */ > if (event->response & FAN_AUDIT) > - audit_fanotify(event->response & ~FAN_AUDIT); > + audit_fanotify(event->response & ~FAN_AUDIT, > + &event->audit_rule); > > pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__, > group, event, ret); > diff --git a/include/linux/audit.h b/include/linux/audit.h > index d6b7d0c7ce43..31086a72e32a 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -14,6 +14,7 @@ > #include <linux/audit_arch.h> > #include <uapi/linux/audit.h> > #include <uapi/linux/netfilter/nf_tables.h> > +#include <uapi/linux/fanotify.h> > > #define AUDIT_INO_UNSET ((unsigned long)-1) > #define AUDIT_DEV_UNSET ((dev_t)-1) > @@ -416,7 +417,7 @@ extern void __audit_log_capset(const struct cred *new, > const struct cred *old); extern void __audit_mmap_fd(int fd, int flags); > extern void __audit_openat2_how(struct open_how *how); > extern void __audit_log_kern_module(char *name); > -extern void __audit_fanotify(u32 response); > +extern void __audit_fanotify(u32 response, struct > fanotify_response_info_audit_rule *friar); extern void > __audit_tk_injoffset(struct timespec64 offset); > extern void __audit_ntp_log(const struct audit_ntp_data *ad); > extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int > nentries, @@ -523,10 +524,10 @@ static inline void > audit_log_kern_module(char *name) __audit_log_kern_module(name); > } > > -static inline void audit_fanotify(u32 response) > +static inline void audit_fanotify(u32 response, struct > fanotify_response_info_audit_rule *friar) { > if (!audit_dummy_context()) > - __audit_fanotify(response); > + __audit_fanotify(response, friar); > } > > static inline void audit_tk_injoffset(struct timespec64 offset) > @@ -679,7 +680,7 @@ static inline void audit_log_kern_module(char *name) > { > } > > -static inline void audit_fanotify(u32 response) > +static inline void audit_fanotify(u32 response, struct > fanotify_response_info_audit_rule *friar) { } > > static inline void audit_tk_injoffset(struct timespec64 offset) > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index d1fb821de104..8d523066d81f 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -64,6 +64,7 @@ > #include <uapi/linux/limits.h> > #include <uapi/linux/netfilter/nf_tables.h> > #include <uapi/linux/openat2.h> // struct open_how > +#include <uapi/linux/fanotify.h> > > #include "audit.h" > > @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) > context->type = AUDIT_KERN_MODULE; > } > > -void __audit_fanotify(u32 response) > +void __audit_fanotify(u32 response, struct > fanotify_response_info_audit_rule *friar) { > - audit_log(audit_context(), GFP_KERNEL, > - AUDIT_FANOTIFY, "resp=%u", response); > + struct audit_context *ctx = audit_context(); > + struct audit_buffer *ab; > + char numbuf[12]; > + > + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { > + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > + "resp=%u fan_type=%u fan_info=3F subj_trust=2 obj_trust=2", > + response, FAN_RESPONSE_INFO_NONE); > + return; > + } > + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY); > + if (ab) { > + audit_log_format(ab, "resp=%u fan_type=%u fan_info=", > + response, friar->hdr.type); > + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number); > + audit_log_n_hex(ab, numbuf, sizeof(numbuf)); I don't think it needs to be converted to ascii and then hexencoded. As Paul said, probably %X is all we need here. -Steve > + audit_log_format(ab, " subj_trust=%u obj_trust=%u", > + friar->subj_trust, friar->obj_trust); > + audit_log_end(ab); > + } > } > > void __audit_tk_injoffset(struct timespec64 offset)