On Wed, Jan 25, 2023 at 5:11 PM Richard Guy Briggs [off-list ref] wrote:

On 2023-01-20 13:58, Paul Moore wrote:

quoted

On Tue, Jan 17, 2023 at 4:14 PM Richard Guy Briggs [off-list ref] wrote:

quoted

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.

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 [off-list ref].

The {subj,obj}_trust values are {0,1,2}, corresponding to no, yes, unknown.

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 <redacted>
Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 (local)
Signed-off-by: Richard Guy Briggs <redacted>
---
 fs/notify/fanotify/fanotify.c |  3 ++-
 include/linux/audit.h         |  9 +++++----
 kernel/auditsc.c              | 16 +++++++++++++---
 3 files changed, 20 insertions(+), 8 deletions(-)

...

quoted

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d1fb821de104..3133c4175c15 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c

@@ -2877,10 +2878,19 @@ 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);
+       /* {subj,obj}_trust values are {0,1,2}: no,yes,unknown */
+       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;
+       }
+       audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY,
+                 "resp=%u fan_type=%u fan_info=%X subj_trust=%u obj_trust=%u",
+                 response, friar->hdr.type, friar->rule_number,
+                 friar->subj_trust, friar->obj_trust);
 }

The only thing that comes to mind might be to convert the if-return
into a switch statement to make it a bit cleaner and easier to patch
in the future, but that is soooo far removed from any real concern
that I debated even mentioning it.  I only bring it up in case the
"3F" discussion results in a respin, and even then I'm not going to
hold my ACK over something as silly as a if-return vs switch.

For clarity, this is what I was thinking:

void __audit_fanontify(...)
{
  switch (type) {
  case FAN_RESPONSE_INFO_NONE:
    audit_log(...);
    break;
  default:
    audit_log(...);
  }
}

I agree that would be cleaner ...

As I said, the "3F" concern of Steve is really the only thing I would
bother respinning for, my other comments were just passing
observations.

-- 
paul-moore.com