| Message ID | 1382626250-15676-1-git-send-email-christoph.paasch@uclouvain.be |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote: > f is set to *arg in route4_change at the beginning, which points to a > route4_filter in the hash-table (gotten through route4_get, called by > tc_ctl_filter). If the alloc of head fails, we should not goto errout, > because this will free f and thus freed memory will be referenced by > the hash-table. > Only later the pointer f will change to an allocated route4_filter. > > This patch returns err if the allocation of head fails as f has not yet > been allocated inside route4_change. > > Seems the code has been like this since Linus's original git-commit. > > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> > --- > net/sched/cls_route.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c > index 37da567..f17c67f 100644 > --- a/net/sched/cls_route.c > +++ b/net/sched/cls_route.c > @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, > if (head == NULL) { > head = kzalloc(sizeof(struct route4_head), GFP_KERNEL); > if (head == NULL) > - goto errout; > + return err; > > tcf_tree_lock(tp); > tp->root = head; I see no bug here, you missed the "goto reinsert;" Guys, if there are no bugs, could we calm down ? Thanks ! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24/10/13 - 07:54:33, Eric Dumazet wrote: > On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote: > > f is set to *arg in route4_change at the beginning, which points to a > > route4_filter in the hash-table (gotten through route4_get, called by > > tc_ctl_filter). If the alloc of head fails, we should not goto errout, > > because this will free f and thus freed memory will be referenced by > > the hash-table. > > Only later the pointer f will change to an allocated route4_filter. > > > > This patch returns err if the allocation of head fails as f has not yet > > been allocated inside route4_change. > > > > Seems the code has been like this since Linus's original git-commit. > > > > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> > > --- > > net/sched/cls_route.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c > > index 37da567..f17c67f 100644 > > --- a/net/sched/cls_route.c > > +++ b/net/sched/cls_route.c > > @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, > > if (head == NULL) { > > head = kzalloc(sizeof(struct route4_head), GFP_KERNEL); > > if (head == NULL) > > - goto errout; > > + return err; > > > > tcf_tree_lock(tp); > > tp->root = head; > > I see no bug here, you missed the "goto reinsert;" Ups - sorry... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-10-24 at 16:59 +0200, Christoph Paasch wrote: > On 24/10/13 - 07:54:33, Eric Dumazet wrote: > > I see no bug here, you missed the "goto reinsert;" > > Ups - sorry... > Yeah, trying to find bugs in Alexey code is really tricky ;) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24/10/13 - 08:05:57, Eric Dumazet wrote: > On Thu, 2013-10-24 at 16:59 +0200, Christoph Paasch wrote: > > On 24/10/13 - 07:54:33, Eric Dumazet wrote: > > > > I see no bug here, you missed the "goto reinsert;" > > > > Ups - sorry... > > > > Yeah, trying to find bugs in Alexey code is really tricky ;) Yeah :) Instead of the goto, a simple if {} else {} would have made it more readable IMO. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index 37da567..f17c67f 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, if (head == NULL) { head = kzalloc(sizeof(struct route4_head), GFP_KERNEL); if (head == NULL) - goto errout; + return err; tcf_tree_lock(tp); tp->root = head;
f is set to *arg in route4_change at the beginning, which points to a route4_filter in the hash-table (gotten through route4_get, called by tc_ctl_filter). If the alloc of head fails, we should not goto errout, because this will free f and thus freed memory will be referenced by the hash-table. Only later the pointer f will change to an allocated route4_filter. This patch returns err if the allocation of head fails as f has not yet been allocated inside route4_change. Seems the code has been like this since Linus's original git-commit. Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> --- net/sched/cls_route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)