On Mon, Sep 30, 2013 at 10:04:25PM +0200, Mathias Krause wrote: > Using the nlmsg_len member of the netlink header to test if the message > is valid is wrong as it includes the size of the netlink header itself.
Normally, yes. The original kaudit unicast socket sends up messages with nlmsg_len set to the payload length rather than the entire message length. This breaks the convention used by netlink. This is set in audit_log_end(). (audit_make_reply looks like it needs a revisit...) The existing auditd daemon assumes this breakage. Fixing this would require co-ordinating a change in the established protocol between kaudit kernel code and auditd userspace code. > Thereby allowing to send short netlink messages that pass those checks. > > Use nlmsg_len() instead to test for the right message length. The result > of nlmsg_len() is guaranteed to be non-negative as the netlink message > already passed the checks of nlmsg_ok(). Since both of these are downward-headed messages, you are correct. > Also switch to min_t() to please checkpatch.pl. Thank you for the catch. > Cc: Al Viro <[email protected]> > Cc: Eric Paris <[email protected]> > Cc: [email protected] # v2.6.6+ for the 1st hunk, v2.6.23+ for the 2nd > Signed-off-by: Mathias Krause <[email protected]> > --- > kernel/audit.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index e237712..10c7263 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -671,7 +671,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct > nlmsghdr *nlh) > &status_set, sizeof(status_set)); > break; > case AUDIT_SET: > - if (nlh->nlmsg_len < sizeof(struct audit_status)) > + if (nlmsg_len(nlh) < sizeof(struct audit_status)) > return -EINVAL; > status_get = (struct audit_status *)data; > if (status_get->mask & AUDIT_STATUS_ENABLED) { > @@ -833,7 +833,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct > nlmsghdr *nlh) > > memset(&s, 0, sizeof(s)); > /* guard against past and future API changes */ > - memcpy(&s, data, min(sizeof(s), (size_t)nlh->nlmsg_len)); > + memcpy(&s, data, min_t(size_t, sizeof(s), nlmsg_len(nlh))); > if ((s.enabled != 0 && s.enabled != 1) || > (s.log_passwd != 0 && s.log_passwd != 1)) > return -EINVAL; > -- > 1.7.10.4 > > -- > Linux-audit mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <[email protected]> Senior Software Engineer Kernel Security AMER ENG Base Operating Systems Remote, Ottawa, Canada Voice: +1.647.777.2635 Internal: (81) 32635 Alt: +1.613.693.0684x3545 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
