[PKT_SCHED] act_gact: division by zero

[PKT_SCHED] act_gact: division by zero

[Nordlund Kim (Nokia-NET/Helsinki)]

tc qdisc add dev eth1 handle ffff: ingress
tc filter add dev eth1 protocol ip parent ffff: pref 99 basic \
   flowid 1:1 action pass random determ drop 0
                                             ^
the above cause a division by zero in the kernel on the first
packet in.

Signed-off-by: Kim Nordlund <kim.nordlund@nokia.com>

diff -rub linux-2.6.19-orig/net/sched/act_gact.c linux/net/sched/act_gact.c
--- linux-2.6.19-orig/net/sched/act_gact.c	2006-11-29 23:57:37.000000000 +0200
+++ linux/net/sched/act_gact.c	2006-11-30 13:22:37.000000000 +0200
@@ -111,7 +111,7 @@
 	if (tb[TCA_GACT_PROB-1] != NULL) {
 		struct tc_gact_p *p_parm = RTA_DATA(tb[TCA_GACT_PROB-1]);
 		gact->tcfg_paction = p_parm->paction;
-		gact->tcfg_pval    = p_parm->pval;
+		gact->tcfg_pval    = p_parm->pval ? : 1;
 		gact->tcfg_ptype   = p_parm->ptype;
 	}
 #endif

Re: [PKT_SCHED] act_gact: division by zero

[Patrick McHardy]

Nordlund Kim (Nokia-NET/Helsinki) wrote:
> tc qdisc add dev eth1 handle ffff: ingress
> tc filter add dev eth1 protocol ip parent ffff: pref 99 basic \
>    flowid 1:1 action pass random determ drop 0
>                                              ^
> the above cause a division by zero in the kernel on the first
> packet in.
> 
> Signed-off-by: Kim Nordlund <kim.nordlund@nokia.com>
> 
> diff -rub linux-2.6.19-orig/net/sched/act_gact.c linux/net/sched/act_gact.c
> --- linux-2.6.19-orig/net/sched/act_gact.c	2006-11-29 23:57:37.000000000 +0200
> +++ linux/net/sched/act_gact.c	2006-11-30 13:22:37.000000000 +0200
> @@ -111,7 +111,7 @@
>  	if (tb[TCA_GACT_PROB-1] != NULL) {
>  		struct tc_gact_p *p_parm = RTA_DATA(tb[TCA_GACT_PROB-1]);
>  		gact->tcfg_paction = p_parm->paction;
> -		gact->tcfg_pval    = p_parm->pval;
> +		gact->tcfg_pval    = p_parm->pval ? : 1;


I think it should reject an invalid configuration or handle
the zero case correctly by not dividing.

Re: [PKT_SCHED] act_gact: division by zero

[Nordlund Kim (Nokia-NET/Helsinki)]

On Thu, 30 Nov 2006, ext Patrick McHardy wrote:
> I think it should reject an invalid configuration or handle
> the zero case correctly by not dividing.

You are correct. Not returning -EINVAL, because someone might
want to use the value zero in some future gact_prob algorithm?

Signed-off-by: Kim Nordlund <kim.nordlund@nokia.com>

diff -rub linux-2.6.19-orig/net/sched/act_gact.c linux/net/sched/act_gact.c
--- linux-2.6.19-orig/net/sched/act_gact.c	2006-11-29 23:57:37.000000000 +0200
+++ linux/net/sched/act_gact.c	2006-11-30 15:33:12.000000000 +0200
@@ -48,14 +48,14 @@
 #ifdef CONFIG_GACT_PROB
 static int gact_net_rand(struct tcf_gact *gact)
 {
-	if (net_random() % gact->tcfg_pval)
+	if (!gact->tcfg_pval || net_random() % gact->tcfg_pval)
 		return gact->tcf_action;
 	return gact->tcfg_paction;
 }
 
 static int gact_determ(struct tcf_gact *gact)
 {
-	if (gact->tcf_bstats.packets % gact->tcfg_pval)
+	if (!gact->tcfg_pval || gact->tcf_bstats.packets % gact->tcfg_pval)
 		return gact->tcf_action;
 	return gact->tcfg_paction;
 }

Re: [PKT_SCHED] act_gact: division by zero

[Patrick McHardy]

Nordlund Kim (Nokia-NET/Helsinki) wrote:
> On Thu, 30 Nov 2006, ext Patrick McHardy wrote:
> 
>>I think it should reject an invalid configuration or handle
>>the zero case correctly by not dividing.
> 
> 
> You are correct. Not returning -EINVAL, because someone might
> want to use the value zero in some future gact_prob algorithm?

That looks good, thanks.

Re: [PKT_SCHED] act_gact: division by zero

[David Miller]

From: "Nordlund Kim (Nokia-NET/Helsinki)" <Kim.Nordlund@nokia.com>
Date: Thu, 30 Nov 2006 15:51:04 +0200 (EET)

> 
> On Thu, 30 Nov 2006, ext Patrick McHardy wrote:
> > I think it should reject an invalid configuration or handle
> > the zero case correctly by not dividing.
> 
> You are correct. Not returning -EINVAL, because someone might
> want to use the value zero in some future gact_prob algorithm?
> 
> Signed-off-by: Kim Nordlund <kim.nordlund@nokia.com>

Applied, thanks Kim.