Re: [PATCH v4 4/4] fanotify,audit: deliver fan_info as a hex-encoded string

[Richard Guy Briggs <rgb@redhat.com>]

On 2022-08-10 15:15, Steve Grubb wrote:
> Hell Richard,

That's quite an introduction!  ;-)

> On Tuesday, August 9, 2022 1:22:55 PM EDT Richard Guy Briggs wrote:
> > 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.
> > 
> > Sample record:
> >   type=FANOTIFY msg=audit(1659730979.839:284): resp=1 fan_type=0
> > fan_info=3F
> 
> I compiled a new kernel and run old user space on this. The above event is 
> exactly what I see in my audit logs. Why the fan_info=3F? I really would have 
> expected 0. What if the actual rule number was 63? I think this will work 
> better to leave everything 0 with old user space.

Well, if it is to be consistently hex encoded, that corresponds to "?"
if it is to be interpreted as a string.  Since the fan_type is 0,
fan_info would be invalid, so a value of 0 would be entirely reasonable,
hex encoded to fan_info=00.  It could also be hex encoded to the string
"(none)".  If you wanted "0" for fan_type=FAN_RESPONSE_INFO_AUDIT_RULE,
that would be fan_info=30 if it were interpreted as a string, or
arguably 3F for an integer of rule (decimal) 63.  Ultimately, fan_type
should determine how fan_info's hex encoded value should be interpreted.

But ultimately, the point of this patch is to hex encode the fan_info
field value.

> -Steve
>  
> > Suggested-by: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/auditsc.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index f000fec52360..0f747015c577 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2908,22 +2908,36 @@ void __audit_fanotify(u32 response, size_t len,
> > char *buf)
> > 
> >  	if (!(len && buf)) {
> >  		audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY,
> > -			  "resp=%u fan_type=0 fan_info=?", response);
> > +			  "resp=%u fan_type=0 fan_info=3F", response); /* "?" */
> >  		return;
> >  	}
> >  	while (c >= sizeof(struct fanotify_response_info_header)) {
> > +		struct audit_context *ctx = audit_context();
> > +		struct audit_buffer *ab;
> > +
> >  		friar = (struct fanotify_response_info_audit_rule *)buf;
> >  		switch (friar->hdr.type) {
> >  		case FAN_RESPONSE_INFO_AUDIT_RULE:
> >  			if (friar->hdr.len < sizeof(*friar)) {
> > -				audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY,
> > -					  "resp=%u fan_type=%u fan_info=(incomplete)",
> > -					  response, friar->hdr.type);
> > +				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);
> > +#define INCOMPLETE "(incomplete)"
> > +					audit_log_n_hex(ab, INCOMPLETE, sizeof(INCOMPLETE));
> > +					audit_log_end(ab);
> > +				}
> >  				return;
> >  			}
> > -			audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY,
> > -				  "resp=%u fan_type=%u fan_info=%u",
> > -				  response, friar->hdr.type, friar->audit_rule);
> > +			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);
> > +				audit_log_n_hex(ab, (char *)&friar->audit_rule,
> > +						sizeof(friar->audit_rule));
> > +				audit_log_end(ab);
> > +
> > +			}
> >  		}
> >  		c -= friar->hdr.len;
> >  		ib += friar->hdr.len;
> 
> 
> 
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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