lkml.org 
[lkml]   [2022]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v4 4/4] fanotify,audit: deliver fan_info as a hex-encoded string
    Date
    Hello Richard,

    Although I have it working, I have some comments below that might improve
    things.

    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
    >
    > 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));

    One thing to point out, the structure has a member audit_rule. It is
    probably better to call it rule_number. This is because it has nothing to
    do with any actual audit rule. It is a rule number meant to be recorded by
    the audit system.

    Also, that member is a __u32 type. Hex encoding that directly gives back a
    __u32 when decoded - which is a bit unexpected since everything else is
    strings. It would be better to convert the u32 to a base 10 string and then
    hex encode that. A buffer of 12 bytes should be sufficient.

    Thanks,
    -Steve

    > + audit_log_end(ab);
    > +
    > + }
    > }
    > c -= friar->hdr.len;
    > ib += friar->hdr.len;




    \
     
     \ /
      Last update: 2022-08-16 15:55    [W:4.080 / U:1.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site