Re: [PATCH 1/3] [PKT_SCHED]: Fix illegal memory dereferences when dumping actions

2006-07-05 Thread Thomas Graf 
* Patrick McHardy [EMAIL PROTECTED] 2006-07-05 01:42
 Thomas Graf wrote:
  @@ -834,7 +833,7 @@ tc_dump_action(struct sk_buff *skb, stru
  a.ops = a_o;
   
  if (a_o-walk == NULL) {
  -   printk(tc_dump_action: %s !capable of dumping table\n, kind);
  +   printk(tc_dump_action: %s !capable of dumping table\n, 
  a_o-kind);
  goto rtattr_failure;
  }
 
 Can't we just get rid of these printks? This seems like a good
 opportunity.

I agree, since this patch is definitely a stable candidate I
wanted it to stay as small as possible.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] [PKT_SCHED]: Fix illegal memory dereferences when dumping actions

2006-07-04 Thread Thomas Graf 
The TCA_ACT_KIND attribute is used without checking its
availability when dumping actions therefore leading to a
value of 0x4 being dereferenced.

The use of strcmp() in tc_lookup_action_n() isn't safe
when fed with string from an attribute without enforcing
proper NUL termination.

Both bugs can be triggered with malformed netlink message
and don't require any privileges.

Signed-off-by: Thomas Graf [EMAIL PROTECTED]

Index: net-2.6.git/net/sched/act_api.c
===
--- net-2.6.git.orig/net/sched/act_api.c
+++ net-2.6.git/net/sched/act_api.c
@@ -776,7 +776,7 @@ replay:
return ret;
 }
 
-static char *
+static struct rtattr *
 find_dump_kind(struct nlmsghdr *n)
 {
struct rtattr *tb1, *tb2[TCA_ACT_MAX+1];
@@ -804,7 +804,7 @@ find_dump_kind(struct nlmsghdr *n)
return NULL;
kind = tb2[TCA_ACT_KIND-1];
 
-   return (char *) RTA_DATA(kind);
+   return kind;
 }
 
 static int
@@ -817,16 +817,15 @@ tc_dump_action(struct sk_buff *skb, stru
struct tc_action a;
int ret = 0;
struct tcamsg *t = (struct tcamsg *) NLMSG_DATA(cb-nlh);
-   char *kind = find_dump_kind(cb-nlh);
+   struct rtattr *kind = find_dump_kind(cb-nlh);
 
if (kind == NULL) {
printk(tc_dump_action: action bad kind\n);
return 0;
}
 
-   a_o = tc_lookup_action_n(kind);
+   a_o = tc_lookup_action(kind);
if (a_o == NULL) {
-   printk(failed to find %s\n, kind);
return 0;
}
 
@@ -834,7 +833,7 @@ tc_dump_action(struct sk_buff *skb, stru
a.ops = a_o;
 
if (a_o-walk == NULL) {
-   printk(tc_dump_action: %s !capable of dumping table\n, kind);
+   printk(tc_dump_action: %s !capable of dumping table\n, 
a_o-kind);
goto rtattr_failure;
}
 

--

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] [PKT_SCHED]: Fix illegal memory dereferences when dumping actions

2006-07-04 Thread jamal 
On Wed, 2006-05-07 at 00:00 +0200, Thomas Graf wrote:
 plain text document attachment (act_fix_dump_null_deref)
 The TCA_ACT_KIND attribute is used without checking its
 availability when dumping actions therefore leading to a
 value of 0x4 being dereferenced.
 
 The use of strcmp() in tc_lookup_action_n() isn't safe
 when fed with string from an attribute without enforcing
 proper NUL termination.
 
 Both bugs can be triggered with malformed netlink message
 and don't require any privileges.
 
 Signed-off-by: Thomas Graf [EMAIL PROTECTED]
 

Good catch.

Acked-by: Jamal Hadi Salim [EMAIL PROTECTED]


cheers,
jamal

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] [PKT_SCHED]: Fix illegal memory dereferences when dumping actions

2006-07-04 Thread jamal 
On Wed, 2006-05-07 at 01:42 +0200, Patrick McHardy wrote:
 Thomas Graf wrote:
  if (a_o-walk == NULL) {
  -   printk(tc_dump_action: %s !capable of dumping table\n, kind);
  +   printk(tc_dump_action: %s !capable of dumping table\n, 
  a_o-kind);
  goto rtattr_failure;
  }
 
 Can't we just get rid of these printks? This seems like a good
 opportunity.
 

perhaps convert to DPRINTKs instead

cheers,
jamal


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html