[PATCH] Audit: Use nlmsg_len() and aggregate into one place.

[Eric Paris]

2010/6/29 Tetsuo Handa <penguin-kernel at i-love.sakura.ne.jp>:
> [PATCH] Audit: Use nlmsg_len() and aggregate into one place.
>
> AUDIT_SET and AUDIT_TTY_SET should use nlmsg_len() rather than nlh->nlmsg_len .
>
> AUDIT_MAKE_EQUIV should do "signed int" comparison rather than "unsigned int"
> comparison because nlmsg_len() (which is nlh->nlmsg_len - NLMSG_HDRLEN) might
> return negative value.
>
> Also aggregate several nlmsg_len(nlh) calls into one place on the assumption
> that nlh->nlmsg_len does not change.
>
> Totally untested. Please review and test.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>

Acked-by: Eric Paris <eparis at redhat.com>

> ---
>  kernel/audit.c |   20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> --- linux-2.6.35-rc3.orig/kernel/audit.c
> +++ linux-2.6.35-rc3/kernel/audit.c
> @@ -662,6 +662,7 @@ static int audit_receive_msg(struct sk_b
>        struct audit_sig_info   *sig_data;
>        char                    *ctx = NULL;
>        u32                     len;
> +       int data_len;
>
>        err = audit_netlink_ok(skb, msg_type);
>        if (err)
> @@ -684,6 +685,7 @@ static int audit_receive_msg(struct sk_b
>        sid  = NETLINK_CB(skb).sid;
>        seq  = nlh->nlmsg_seq;
>        data = NLMSG_DATA(nlh);
> +       data_len = nlmsg_len(nlh);
>
>        switch (msg_type) {
>        case AUDIT_GET:
> @@ -698,7 +700,7 @@ static int audit_receive_msg(struct sk_b
>                                 &status_set, sizeof(status_set));
>                break;
>        case AUDIT_SET:
> -               if (nlh->nlmsg_len < sizeof(struct audit_status))
> +               if (data_len < sizeof(struct audit_status))
>                        return -EINVAL;
>                status_get   = (struct audit_status *)data;
>                if (status_get->mask & AUDIT_STATUS_ENABLED) {
> @@ -759,7 +761,7 @@ static int audit_receive_msg(struct sk_b
>                                int size;
>
>                                audit_log_format(ab, " msg=");
> -                               size = nlmsg_len(nlh);
> +                               size = data_len;
>                                if (size > 0 &&
>                                    ((unsigned char *)data)[size - 1] == '\0')
>                                        size--;
> @@ -771,7 +773,7 @@ static int audit_receive_msg(struct sk_b
>                break;
>        case AUDIT_ADD:
>        case AUDIT_DEL:
> -               if (nlmsg_len(nlh) < sizeof(struct audit_rule))
> +               if (data_len < sizeof(struct audit_rule))
>                        return -EINVAL;
>                if (audit_enabled == AUDIT_LOCKED) {
>                        audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE, pid,
> @@ -785,12 +787,12 @@ static int audit_receive_msg(struct sk_b
>                /* fallthrough */
>        case AUDIT_LIST:
>                err = audit_receive_filter(msg_type, NETLINK_CB(skb).pid,
> -                                          uid, seq, data, nlmsg_len(nlh),
> +                                          uid, seq, data, data_len,
>                                           loginuid, sessionid, sid);
>                break;
>        case AUDIT_ADD_RULE:
>        case AUDIT_DEL_RULE:
> -               if (nlmsg_len(nlh) < sizeof(struct audit_rule_data))
> +               if (data_len < sizeof(struct audit_rule_data))
>                        return -EINVAL;
>                if (audit_enabled == AUDIT_LOCKED) {
>                        audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE, pid,
> @@ -804,7 +806,7 @@ static int audit_receive_msg(struct sk_b
>                /* fallthrough */
>        case AUDIT_LIST_RULES:
>                err = audit_receive_filter(msg_type, NETLINK_CB(skb).pid,
> -                                          uid, seq, data, nlmsg_len(nlh),
> +                                          uid, seq, data, data_len,
>                                           loginuid, sessionid, sid);
>                break;
>        case AUDIT_TRIM:
> @@ -819,11 +821,11 @@ static int audit_receive_msg(struct sk_b
>        case AUDIT_MAKE_EQUIV: {
>                void *bufp = data;
>                u32 sizes[2];
> -               size_t msglen = nlmsg_len(nlh);
> +               size_t msglen = data_len;
>                char *old, *new;
>
>                err = -EINVAL;
> -               if (msglen < 2 * sizeof(u32))
> +               if (data_len < 2 * sizeof(u32))
>                        break;
>                memcpy(sizes, bufp, 2 * sizeof(u32));
>                bufp += 2 * sizeof(u32);
> @@ -900,7 +902,7 @@ static int audit_receive_msg(struct sk_b
>                struct audit_tty_status *s;
>                struct task_struct *tsk;
>
> -               if (nlh->nlmsg_len < sizeof(struct audit_tty_status))
> +               if (data_len < sizeof(struct audit_tty_status))
>                        return -EINVAL;
>                s = data;
>                if (s->enabled != 0 && s->enabled != 1)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>