sch_dsmark bug

sch_dsmark bug

[Hugo Santos]

Hi there,

I tried to contact the person whose name is in the source file but with
no success. While developing some code that handles with kernel's TC
objects direcly (by netlink) i found a small problem with dsmark (maybe
other scheds have it too, but i haven't checked). This is in linux 2.6.4
(there is no change in this context in 2.6.5); in dsmark_init, if opt is
NULL, the module will crash and TC will get stuck. the module will have a
ref count of 1 so i can't unload it, and i can't change any other TC
stuff since the netlink socket gets stuck when dsmark crashes. So, a simple

if (!opt) return -EINVAL;

would prevent such problems.

If you prefer i may send a patch, but since it's only a one line patch i didn't
send one.

Best regards,
Hugo Santos

Re: sch_dsmark bug

[jamal]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]


Looks legit nit. Why are you sending empty opts anyways? ;->
patchlet attached - you should have sent one ;->
Maybe you can inspect other qdiscs.

cheers,
jamal

On Sun, 2004-04-11 at 08:43, Hugo Santos wrote:
> Hi there,
> 
> I tried to contact the person whose name is in the source file but with
> no success. While developing some code that handles with kernel's TC
> objects direcly (by netlink) i found a small problem with dsmark (maybe
> other scheds have it too, but i haven't checked). This is in linux 2.6.4
> (there is no change in this context in 2.6.5); in dsmark_init, if opt is
> NULL, the module will crash and TC will get stuck. the module will have a
> ref count of 1 so i can't unload it, and i can't change any other TC
> stuff since the netlink socket gets stuck when dsmark crashes. So, a simple
> 
> if (!opt) return -EINVAL;
> 
> would prevent such problems.
> 
> If you prefer i may send a patch, but since it's only a one line patch i didn't
> send one.
> 
> Best regards,
> Hugo Santos
> 
> 
> 
> 
> 

[-- Attachment #2: p1 --]
[-- Type: text/plain, Size: 353 bytes --]

--- sch_dsmark.c	2004/04/11 19:39:16	1.1
+++ sch_dsmark.c	2004/04/11 19:39:41
@@ -321,6 +321,7 @@
 	struct rtattr *tb[TCA_DSMARK_MAX];
 	__u16 tmp;
 
+	if (!opt) return -EINVAL;
 	DPRINTK("dsmark_init(sch %p,[qdisc %p],opt %p)\n",sch,p,opt);
 	if (rtattr_parse(tb,TCA_DSMARK_MAX,RTA_DATA(opt),RTA_PAYLOAD(opt)) < 0 ||
 	    !tb[TCA_DSMARK_INDICES-1] ||

Re: sch_dsmark bug

[Sven Schuster]

[-- Attachment #1.1: Type: text/plain, Size: 465 bytes --]

On Sun, Apr 11, 2004 at 03:43:03PM -0400, jamal told us:
> 
> Looks legit nit. Why are you sending empty opts anyways? ;->
> patchlet attached - you should have sent one ;->
> Maybe you can inspect other qdiscs.
> 
> cheers,
> jamal
> 

I took a look at the others and found this ones...


Sven

-- 
Linux zion 2.6.5 #1 Sun Apr 4 19:56:55 CEST 2004 i686 athlon i386 GNU/Linux
 00:43:25  up 7 days,  1:27,  2 users,  load average: 0.02, 0.05, 0.01

[-- Attachment #1.2: sched.patch --]
[-- Type: text/plain, Size: 1011 bytes --]

--- net/sched/sch_cbq.c.orig	2004-04-12 00:24:08.123743284 +0200
+++ net/sched/sch_cbq.c	2004-04-12 00:40:30.186534165 +0200
@@ -1399,7 +1399,8 @@ static int cbq_init(struct Qdisc *sch, s
 	struct rtattr *tb[TCA_CBQ_MAX];
 	struct tc_ratespec *r;
 
-	if (rtattr_parse(tb, TCA_CBQ_MAX, RTA_DATA(opt), RTA_PAYLOAD(opt)) < 0 ||
+	if (!opt || 
+	    rtattr_parse(tb, TCA_CBQ_MAX, RTA_DATA(opt), RTA_PAYLOAD(opt)) < 0 ||
 	    tb[TCA_CBQ_RTAB-1] == NULL || tb[TCA_CBQ_RATE-1] == NULL ||
 	    RTA_PAYLOAD(tb[TCA_CBQ_RATE-1]) < sizeof(struct tc_ratespec))
 		return -EINVAL;
--- net/sched/sch_csz.c.orig	2004-04-12 00:24:08.126743018 +0200
+++ net/sched/sch_csz.c	2004-04-12 00:36:16.195184499 +0200
@@ -763,6 +763,8 @@ static int csz_init(struct Qdisc *sch, s
 	struct tc_csz_qopt *qopt;
 	int    i;
 
+	if (!opt)
+		return -EINVAL;
 	rtattr_parse(tb, TCA_CSZ_PTAB, RTA_DATA(opt), RTA_PAYLOAD(opt));
 	if (tb[TCA_CSZ_PARMS-1] == NULL ||
 	    RTA_PAYLOAD(tb[TCA_CSZ_PARMS-1]) < sizeof(*qopt))

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]