* [PATCH] Audit: Use nlmsg_len() and aggregate into one place.
@ 2010-06-30 2:09 Tetsuo Handa
2010-06-30 16:31 ` Eric Paris
0 siblings, 1 reply; 2+ messages in thread
From: Tetsuo Handa @ 2010-06-30 2:09 UTC (permalink / raw)
To: linux-audit; +Cc: linux-security-module
[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@I-love.SAKURA.ne.jp>
---
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)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Audit: Use nlmsg_len() and aggregate into one place.
2010-06-30 2:09 [PATCH] Audit: Use nlmsg_len() and aggregate into one place Tetsuo Handa
@ 2010-06-30 16:31 ` Eric Paris
0 siblings, 0 replies; 2+ messages in thread
From: Eric Paris @ 2010-06-30 16:31 UTC (permalink / raw)
To: Tetsuo Handa, Al Viro; +Cc: linux-audit, linux-security-module
2010/6/29 Tetsuo Handa <penguin-kernel@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@I-love.SAKURA.ne.jp>
Acked-by: Eric Paris <eparis@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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-06-30 16:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-30 2:09 [PATCH] Audit: Use nlmsg_len() and aggregate into one place Tetsuo Handa
2010-06-30 16:31 ` Eric Paris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.