Yeah I realized after I sent this clearing the bits becomes a problem... Doh
Thanks Bill On May 24, 2013 1:03 PM, "Eric Paris" <[email protected]> wrote: > A memset to 0 is adequate if you want to be able to turn features back > off. You have to know if the 0 or 1 is meaningful. > > Without the mask userland would have to read the features from the > kernel, twiddle whichever bits they want to twiddle, then write the > features back to the kernel. With the mask userspace can specify > which bits they are twiddling and which bits the kernel just shouldn't > touch. So the read before write isn't necessary and removes that > little race. > > I thought it made setting/unsetting bits easier on the userspace side. > > I can get rid of the mask if you think the read/twiddle/write option > is better than set-mask/write > > -Eric > > On Fri, May 24, 2013 at 3:33 PM, William Roberts > <[email protected]> wrote: > > Well when I was adding to the AUDIT MSG types, I felt pretty dirty :-P > > > > I think versioning and bitpacking the features is the right way to go. > > > > Whats the purpose of mask in audit_features struct? > > > > Looks like you use it to indicate which bits userspace intended to set, > > incase the structure wasn't memset to 0? > > > > Bill > > > > > > On Fri, May 24, 2013 at 9:34 AM, Eric Paris <[email protected]> > wrote: > >> > >> On Wed, 2013-05-22 at 19:18 -0700, William Roberts wrote: > >> > This need came about in Samsung's mobile products. Id like to see and > >> > attempt to get this mainstreamed. Any feedback would be greatly > >> > appreciated. Thanks. > >> > >> My first thought is that I just sent a patch to introduce a flexible > >> on/off flipping of bits for audit system features. Rather timely, so we > >> don't have to keep introducing message types. AUDIT_SET/AUDIT_GET > >> certainly weren't well thought out in that way. I'd like your thoughts > >> on those patches (really 1/7 does the heavy lifting 6/7 and 7/7 use > >> it) > >> > >> I thinking porting on top of my series will result in your patch being > >> about 10 lines. > >> > >> I also don't love the name. This isn't about splitting logs, it's about > >> also logging to kmsg. > >> > >> Do we need the Kconfig default? I'd think we should be able to get this > >> set early enough in userspace. Any messages before auditd are going to > >> go to kmsg. I envision that we should be able to set features in the > >> audit.rules so you should be able to turn this on before auditd starts > >> and things would stop going to kmsg. > >> > >> But I don't see a problem with it. If you port on top of this series > >> I'm ok with queuing for 3.11 > >> > >> > On Wed, May 22, 2013 at 7:16 PM, William Roberts > >> > <[email protected]> wrote: > >> > Allow the audit subsystem to send audit events to both the > >> > kernel > >> > message buffer and auditd at the same time. > >> > > >> > Signed-off-by: William Roberts <[email protected]> > >> > --- > >> > include/uapi/linux/audit.h | 11 ++++++++++ > >> > init/Kconfig | 8 +++++++ > >> > kernel/audit.c | 52 > >> > ++++++++++++++++++++++++++++++++++++++++------ > >> > kernel/audit.h | 6 ++++++ > >> > 4 files changed, 71 insertions(+), 6 deletions(-) > >> > > >> > diff --git a/include/uapi/linux/audit.h > >> > b/include/uapi/linux/audit.h > >> > index 75cef3f..0af5d78 100644 > >> > --- a/include/uapi/linux/audit.h > >> > +++ b/include/uapi/linux/audit.h > >> > @@ -68,6 +68,8 @@ > >> > #define AUDIT_MAKE_EQUIV 1015 /* Append to watched > >> > tree */ > >> > #define AUDIT_TTY_GET 1016 /* Get TTY auditing > >> > status */ > >> > #define AUDIT_TTY_SET 1017 /* Set TTY auditing > >> > status */ > >> > +#define AUDIT_LOGSPLIT_GET 1018 /* Get logsplit > >> > status */ > >> > +#define AUDIT_LOGSPLIT_SET 1019 /* Set logsplit > >> > status */ > >> > > >> > #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages > >> > mostly uninteresting to kernel */ > >> > #define AUDIT_USER_AVC 1107 /* We filter this > >> > differently */ > >> > @@ -321,6 +323,10 @@ enum { > >> > #define AUDIT_FAIL_PRINTK 1 > >> > #define AUDIT_FAIL_PANIC 2 > >> > > >> > +/* Audit splitlog options */ > >> > +#define AUDIT_LOGSPLIT_OFF 0 > >> > +#define AUDIT_LOGSPLIT_ON 1 > >> > + > >> > /* distinguish syscall tables */ > >> > #define __AUDIT_ARCH_64BIT 0x80000000 > >> > #define __AUDIT_ARCH_LE 0x40000000 > >> > @@ -374,6 +380,11 @@ struct audit_tty_status { > >> > __u32 log_passwd; /* 1 = enabled, 0 = > >> > disabled */ > >> > }; > >> > > >> > +struct audit_logsplit_status { > >> > + __u32 enabled; /* AUDIT_LOGSPLIT_ON > >> > or > >> > + > >> > AUDIT_LOGSPLIT_OFF */ > >> > +}; > >> > + > >> > /* audit_rule_data supports filter rules with both integer > >> > and string > >> > * fields. It corresponds with AUDIT_ADD_RULE, > >> > AUDIT_DEL_RULE and > >> > * AUDIT_LIST_RULES requests. > >> > diff --git a/init/Kconfig b/init/Kconfig > >> > index 9d3a788..52efca6 100644 > >> > --- a/init/Kconfig > >> > +++ b/init/Kconfig > >> > @@ -286,6 +286,14 @@ config AUDIT_LOGINUID_IMMUTABLE > >> > one to drop potentially dangerous capabilites from > >> > the login tasks, > >> > but may not be backwards compatible with older init > >> > systems. > >> > > >> > +config AUDIT_SPLITLOG > >> > + bool "Split audit messages to userspace and kernel by > >> > default" > >> > + depends on AUDIT > >> > + help > >> > + Setting this to true will cause the audit messages > >> > to be split > >> > + by default to both the kernel message log and a > >> > userspace audit > >> > + daemon if registered. > >> > + > >> > source "kernel/irq/Kconfig" > >> > source "kernel/time/Kconfig" > >> > > >> > diff --git a/kernel/audit.c b/kernel/audit.c > >> > index 21c7fa6..ccd2f60 100644 > >> > --- a/kernel/audit.c > >> > +++ b/kernel/audit.c > >> > @@ -88,6 +88,9 @@ static int audit_default; > >> > /* If auditing cannot proceed, audit_failure selects what > >> > happens. */ > >> > static int audit_failure = AUDIT_FAIL_PRINTK; > >> > > >> > +/* Whether or not logsplit is enabled */ > >> > +static int audit_logsplit = AUDIT_SPLITLOG_INIT; > >> > + > >> > /* > >> > * If audit records are to be written to the netlink socket, > >> > audit_pid > >> > * contains the pid of the auditd process and > >> > audit_nlk_portid contains > >> > @@ -343,6 +346,17 @@ static int audit_set_failure(int state) > >> > return audit_do_config_change("audit_failure", > >> > &audit_failure, state); > >> > } > >> > > >> > +static int audit_set_logsplit(int state) > >> > +{ > >> > + if (state != AUDIT_LOGSPLIT_OFF > >> > + && state != AUDIT_LOGSPLIT_ON) > >> > + return -EINVAL; > >> > + > >> > + return audit_do_config_change("audit_logsplit", > >> > &audit_logsplit, state); > >> > +} > >> > + > >> > + > >> > + > >> > /* > >> > * Queue skbs to be sent to auditd when/if it comes back. > >> > These skbs should > >> > * already have been sent via prink/syslog and so if these > >> > messages are dropped > >> > @@ -361,11 +375,8 @@ static void audit_hold_skb(struct sk_buff > >> > *skb) > >> > kfree_skb(skb); > >> > } > >> > > >> > -/* > >> > - * For one reason or another this nlh isn't getting delivered > >> > to the userspace > >> > - * audit daemon, just send it to printk. > >> > - */ > >> > -static void audit_printk_skb(struct sk_buff *skb) > >> > +/* Just printks the skb, no audit_hold or free of any kind */ > >> > +static void __audit_printk_skb(struct sk_buff *skb) > >> > { > >> > struct nlmsghdr *nlh = nlmsg_hdr(skb); > >> > char *data = nlmsg_data(nlh); > >> > @@ -376,7 +387,14 @@ static void audit_printk_skb(struct > >> > sk_buff *skb) > >> > else > >> > audit_log_lost("printk limit exceeded > >> > \n"); > >> > } > >> > - > >> > +} > >> > +/* > >> > + * For one reason or another this nlh isn't getting delivered > >> > to the userspace > >> > + * audit daemon, just send it to printk. > >> > + */ > >> > +static void audit_printk_skb(struct sk_buff *skb) > >> > +{ > >> > + __audit_printk_skb(skb); > >> > audit_hold_skb(skb); > >> > } > >> > > >> > @@ -590,6 +608,8 @@ static int audit_netlink_ok(struct sk_buff > >> > *skb, u16 msg_type) > >> > case AUDIT_SIGNAL_INFO: > >> > case AUDIT_TTY_GET: > >> > case AUDIT_TTY_SET: > >> > + case AUDIT_LOGSPLIT_GET: > >> > + case AUDIT_LOGSPLIT_SET: > >> > case AUDIT_TRIM: > >> > case AUDIT_MAKE_EQUIV: > >> > if (!capable(CAP_AUDIT_CONTROL)) > >> > @@ -843,7 +863,24 @@ static int audit_receive_msg(struct > >> > sk_buff *skb, struct nlmsghdr *nlh) > >> > spin_unlock(&tsk->sighand->siglock); > >> > break; > >> > } > >> > + case AUDIT_LOGSPLIT_GET: { > >> > + struct audit_logsplit_status s; > >> > + s.enabled = audit_logsplit; > >> > + audit_send_reply(NETLINK_CB(skb).portid, seq, > >> > + AUDIT_LOGSPLIT_GET, 0, 0, &s, > >> > sizeof(s)); > >> > + break; > >> > + } > >> > + case AUDIT_LOGSPLIT_SET: { > >> > + struct audit_logsplit_status *s; > >> > + if (nlh->nlmsg_len < sizeof(struct > >> > audit_logsplit_status)) > >> > + return -EINVAL; > >> > + s = data; > >> > + err = audit_set_logsplit(s->enabled); > >> > + break; > >> > + } > >> > + > >> > default: > >> > + printk(KERN_ERR "Unkown audit command"); > >> > err = -EINVAL; > >> > break; > >> > } > >> > @@ -1670,6 +1707,9 @@ void audit_log_end(struct audit_buffer > >> > *ab) > >> > nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN; > >> > > >> > if (audit_pid) { > >> > + if (audit_logsplit == > >> > AUDIT_LOGSPLIT_ON) > >> > + __audit_printk_skb(ab->skb); > >> > + > >> > skb_queue_tail(&audit_skb_queue, > >> > ab->skb); > >> > wake_up_interruptible(&kauditd_wait); > >> > } else { > >> > diff --git a/kernel/audit.h b/kernel/audit.h > >> > index 1c95131..ef43bb1 100644 > >> > --- a/kernel/audit.h > >> > +++ b/kernel/audit.h > >> > @@ -321,4 +321,10 @@ extern struct list_head > >> > *audit_killed_trees(void); > >> > #define audit_filter_inodes(t,c) AUDIT_DISABLED > >> > #endif > >> > > >> > +#ifdef CONFIG_AUDIT_SPLITLOG > >> > +#define AUDIT_SPLITLOG_INIT AUDIT_LOGSPLIT_ON > >> > +#else > >> > +#define AUDIT_SPLITLOG_INIT AUDIT_LOGSPLIT_OFF > >> > +#endif > >> > + > >> > extern struct mutex audit_cmd_mutex; > >> > -- > >> > 1.8.2.2 > >> > > >> > > >> > > >> > > >> > > >> > -- > >> > Respectfully, > >> > > >> > William C Roberts > >> > > >> > > >> > -- > >> > Linux-audit mailing list > >> > [email protected] > >> > https://www.redhat.com/mailman/listinfo/linux-audit > >> > >> > > > > > > > > -- > > Respectfully, > > > > William C Roberts > > >
-- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
