[PATCH 2/2] audit: use nlmsg_len() to get message payload length
Richard Guy Briggs
rgb at redhat.com
Tue Oct 1 03:51:18 UTC 2013
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 <viro at zeniv.linux.org.uk>
> Cc: Eric Paris <eparis at redhat.com>
> Cc: stable at vger.kernel.org # v2.6.6+ for the 1st hunk, v2.6.23+ for the 2nd
> Signed-off-by: Mathias Krause <minipli at googlemail.com>
> ---
> 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
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs at redhat.com>
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
More information about the Linux-audit
mailing list