Re: [RFC] [PATCH] audit: log module name on init_module

On 2017-01-31 11:07, Paul Moore wrote: > On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > > On 2017-01-31 06:59, Paul Moore wrote: > >> On Thu, Jan 26, 2017 at 4:21 PM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > >> > This adds a new auxiliary record MODULE_INIT to the SYSCALL event. > >> > > >> > We get finit_module for free since it made most sense to hook this in to > >> > load_module(). > >> > > >> > https://github.com/linux-audit/audit-kernel/issues/7 > >> > https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-load-record-f... > >> > >> Consistency nit: capitalize the first letter in the wiki page words > >> (see the existing RFE pages) > >> > >> > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > >> > --- > >> > include/linux/audit.h | 12 ++++++++++++ > >> > include/uapi/linux/audit.h | 1 + > >> > kernel/audit.h | 3 +++ > >> > kernel/auditsc.c | 20 ++++++++++++++++++++ > >> > kernel/module.c | 5 ++++- > >> > 5 files changed, 40 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/include/linux/audit.h b/include/linux/audit.h > >> > index 2be99b2..7bb23d5 100644 > >> > --- a/include/linux/audit.h > >> > +++ b/include/linux/audit.h > >> > @@ -360,6 +360,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm, > >> > const struct cred *old); > >> > extern void __audit_log_capset(const struct cred *new, const struct cred *old); > >> > extern void __audit_mmap_fd(int fd, int flags); > >> > +extern void __audit_module_init(char *name); > >> > > >> > static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) > >> > { > >> > @@ -450,6 +451,12 @@ static inline void audit_mmap_fd(int fd, int flags) > >> > __audit_mmap_fd(fd, flags); > >> > } > >> > > >> > +static inline void audit_module_init(char *name) > >> > +{ > >> > + if (!audit_dummy_context()) > >> > + __audit_module_init(name); > >> > +} > >> > >> More on this below, but I was expecting the function above to named > >> audit_log_kern_module(), or something similar. > > > > Ok fair enough, I had mis-understood your previous point. > > I probably could have been more specific too. > > > Any comment on the new record format? > > Not really, it's just the single field so it's kinda hard to have > anything meaningful to say. We obviously need to worry about the > field name, but I'll let Steve speak to that as that likely means more > to him than it does to me. From my perspective, "name" seems > perfectly reasonable, especially since it is in the context of a > module specific record (no real worries about it being ambiguous). Do you see a need to include module initialization arguments? It sounds potentially useful to me, but also potentially bandwidth-consuming. I have a prototype patch to add the args as one encoded field. Along with the addition of this field is the concern about message lengths and buffer allocations since it is an encoded field that would need twice the length of the argment text to store in the message.