On 2019-04-18 11:37, Paul Moore wrote: > On Thu, Apr 18, 2019 at 11:16 AM Richard Guy Briggs <[email protected]> wrote: > > On 2019-04-18 10:59, Paul Moore wrote: > > > On Mon, Apr 8, 2019 at 11:53 PM Richard Guy Briggs <[email protected]> > > > wrote: > > > > When a process signals the audit daemon (shutdown, rotate, resume, > > > > reconfig) but syscall auditing is not enabled, we still want to know the > > > > identity of the process sending the signal to the audit daemon. > > > > > > > > Move audit_signal_info() out of syscall auditing to general auditing but > > > > create a new function audit_signal_info_syscall() to take care of the > > > > syscall dependent parts for when syscall auditing is enabled. > > > > > > > > Please see the github kernel audit issue > > > > https://github.com/linux-audit/audit-kernel/issues/111 > > > > > > > > Signed-off-by: Richard Guy Briggs <[email protected]> > > > > --- > > > > include/linux/audit.h | 6 ++++++ > > > > kernel/audit.c | 27 +++++++++++++++++++++++++++ > > > > kernel/audit.h | 4 ++-- > > > > kernel/auditsc.c | 19 +++---------------- > > > > kernel/signal.c | 2 +- > > > > 5 files changed, 39 insertions(+), 19 deletions(-) > > > > > > ... > > > > > > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > > > index 1e69d9fe16da..4a22fc3f824f 100644 > > > > --- a/include/linux/audit.h > > > > +++ b/include/linux/audit.h > > > > @@ -226,6 +229,9 @@ static inline unsigned int > > > > audit_get_sessionid(struct task_struct *tsk) > > > > } > > > > > > > > #define audit_enabled AUDIT_OFF > > > > + > > > > +#define audit_signal_info(s, t) AUDIT_OFF > > > > + > > > > > > Should this be AUDIT_DISABLED to preserve the current value/behavior? > > > Technically they should both have a value of zero right now, but since > > > the AUDIT_DISABLED value isn't explicit it seems safer to go with > > > AUDIT_DISABLED. > > > > I did that first, but that symbol was not available when one or both of > > CONFIG_AUDITSYSCALL or CONFIG_AUDIT was off, so I had to change it to > > AUDIT_OFF. I followed the logic to confirm that is what was intended by > > the original code. > > > > When auidit is off, we want to just return zero so it gets skipped > > rather than throwing an error. > > I understand the desire to return zero in that case, I'm not arguing > against that, I'm just not really in love with how these are defined > when CONFIG_AUDIT isn't. Part of it is the AUDIT_DISABLED/AUDIT_OFF > change, part of it is the function being defined as a cpp macro > instead of a dummy function (this of course predates your change). > Based on other comments in this thread it looks like you're looking > into a few things and will likely be respinning this patch, since that > is the case, I would prefer if you changed this to just using simply > "0" as opposed to AUDIT_OFF. > > If you really want to make me happy about this patch, you would also > change this to a dummy function instead of a cpp macro. This is a > style nit, and isn't strictly necessary, but I would appreciate it :)
In fact, I'd started with a dummy function, then changed it to match the style of the rest of the file when I got a pile of unused function complaints from all over the kernel... I'll see what I can do to go back to the dummy function. > > > > diff --git a/kernel/audit.h b/kernel/audit.h > > > > index 958d5b8fc1b3..18a8ae812e9f 100644 > > > > --- a/kernel/audit.h > > > > +++ b/kernel/audit.h > > > > @@ -299,7 +299,7 @@ extern bool audit_tree_match(struct audit_chunk > > > > *chunk, > > > > extern void audit_put_tree(struct audit_tree *tree); > > > > extern void audit_kill_trees(struct audit_context *context); > > > > > > > > -extern int audit_signal_info(int sig, struct task_struct *t); > > > > +extern int audit_signal_info_syscall(struct task_struct *t); > > > > extern void audit_filter_inodes(struct task_struct *tsk, > > > > struct audit_context *ctx); > > > > extern struct list_head *audit_killed_trees(void); > > > > @@ -330,7 +330,7 @@ extern void audit_filter_inodes(struct task_struct > > > > *tsk, > > > > #define audit_tree_path(rule) "" /* never called */ > > > > #define audit_kill_trees(context) BUG() > > > > > > > > -#define audit_signal_info(s, t) AUDIT_DISABLED > > > > +#define audit_signal_info_syscall(t) AUDIT_OFF > > > > > > Similar as above. > > -- > paul moore > www.paul-moore.com - RGB -- Richard Guy Briggs <[email protected]> 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 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
