Re: [PATCH ghak59 V3 2/4] audit: add syscall information to CONFIG_CHANGE records

[Steve Grubb]

On Mon, 14 Jan 2019 17:58:58 -0500
Paul Moore <paul@xxxxxxxxxxxxxx> wrote:

> On Mon, Dec 10, 2018 at 5:18 PM Richard Guy Briggs <rgb@xxxxxxxxxx>
> wrote:
> >
> > Tie syscall information to all CONFIG_CHANGE calls since they are
> > all a result of user actions.

Please don't tie syscall information to this. The syscall will be
sendto. We don't need that information, its implicit. Also, doing this
will possibly wreck things in libauparse. Please test the events with
ausearch --format csv and --format text. IFF the event looks better or
the same should we do this. If stuff disappears, the patch is
breaking things

-Steve


> > Exclude user records from syscall context:
> > Since the function audit_log_common_recv_msg() is shared by a
> > number of AUDIT_CONFIG_CHANGE and the entire range of AUDIT_USER_*
> > record types, and since the AUDIT_CONFIG_CHANGE message type has
> > been converted to a syscall accompanied record type, special-case
> > the AUDIT_USER_* range of messages so they remain standalone
> > records.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/59
> > See: https://github.com/linux-audit/audit-kernel/issues/50
> > Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> > ---
> >  kernel/audit.c          | 27 +++++++++++++++++++--------
> >  kernel/audit_fsnotify.c |  2 +-
> >  kernel/audit_tree.c     |  2 +-
> >  kernel/audit_watch.c    |  2 +-
> >  kernel/auditfilter.c    |  2 +-
> >  5 files changed, 23 insertions(+), 12 deletions(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 0e8026423fbd..a321fea94cc6 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1072,6 +1073,16 @@ static void audit_log_common_recv_msg(struct
> > audit_buffer **ab, u16 msg_type) audit_log_task_context(*ab);
> >  }
> >
> > +static inline void audit_log_user_recv_msg(struct audit_buffer
> > **ab, u16 msg_type) +{
> > +       audit_log_common_recv_msg(NULL, ab, msg_type);
> > +}  
> 
> This makes sense because this is used by "user" records ...
> 
> > +static inline void audit_log_config_change_alt(struct audit_buffer
> > **ab) +{
> > +       audit_log_common_recv_msg(audit_context(), ab,
> > AUDIT_CONFIG_CHANGE); +}  
> 
> ... and I don't believe this makes sense because there is no real
> logical grouping with the callers like there is for
> audit_log_user_recv_msg().
>