On 1 October 2013 05:51, Richard Guy Briggs <[email protected]> wrote: > 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...)
Looks like there are two paths for sending netlink messages to userland. One using audit_send_reply(), correctly filling the nlmsg_len. One using audit_log_start()/audit_log_end() implementing the protocol you're describing. Weird :/ > 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. Looking at the current userland audit netlink code (v2.3.2, lib/netlink.c:adjust_reply) it looks like the library doesn't care much. It just ensures nlmsg_len doesn't exceed the length of the received message and that the message is at least as big as a netlink header. It even expects the regular netlink message rules do apply by using NLMSG_OK() for the test -- so nlmsg_len must be at least as large as a netlink header. >> 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. Regards, Mathias -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
