Re: [PATCH v6 3/3] fanotify,audit: Allow audit to use the full permission event response
On Wed, Jan 18, 2023 at 1:34 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
Hello Richard,
I built a new kernel and tested this with old and new user space. It is
working as advertised. The only thing I'm wondering about is why we have 3F
as the default value when no additional info was sent? Would it be better to
just make it 0?
...
On Tuesday, January 17, 2023 4:14:07 PM EST Richard Guy Briggs
wrote:
> 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;
> + }
(I'm working under the assumption that the "fan_info=3F" in the record
above is what Steve was referring to in his comment.)
I vaguely recall Richard commenting on this in the past, although
maybe not ... my thought is that the "3F" is simply the hex encoded
"?" character in ASCII ('man 7 ascii' is your friend). I suppose the
question is what to do in the FAN_RESPONSE_INFO_NONE case.
Historically when we had a missing field we would follow the "field=?"
pattern, but I don't recall doing that for a field which was
potentially hex encoded, is there an existing case where we use "?"
for a field that is hex encoded? If so, we can swap out the "3F" for
a more obvious "?".
However, another option might be to simply output the current
AUDIT_FANOTIFY record format in the FAN_RESPONSE_INFO_NONE case, e.g.
only "resp=%u". This is a little against the usual guidance of
"fields should not disappear from a record", but considering that
userspace will always need to support the original resp-only format
for compatibility reasons this may be an option.
--
paul-moore.com