On 2023-01-09 19:06, Steve Grubb wrote: > 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. I'll have a closer look, because this wasn't the intended behaviour. > 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... Well, it *is* 13 decimal, expressed as a hex-encoded string as was requested for future-proofing, albeit longer than necessary... > > 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@xxxxxxxxxxxxxx>. > > > > 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@xxxxxxxxxx> > > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > > Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx> > > --- > > 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) > > > > - RGB -- Richard Guy Briggs <rgb@xxxxxxxxxx> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
