| Message ID | 53A15267.7050304@cn.fujitsu.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Duan Jiong <duanj.fnst@cn.fujitsu.com> Date: Wed, 18 Jun 2014 16:48:39 +0800 > tcf_ematch is allocated by kzalloc in function tcf_em_tree_validate(), > so cm_old is always NULL. > > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> Applied, 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
This is right for the first set of filters that is ever configured in em_canid_change(). But what happens in an update case? Does is turn the cm_old reference to a memory leak then? Or do we always get a brand new "struct tcf_ematch *m" here?? Regards, Oliver On 22.06.2014 00:40, David Miller wrote: > From: Duan Jiong <duanj.fnst@cn.fujitsu.com> > Date: Wed, 18 Jun 2014 16:48:39 +0800 > >> tcf_ematch is allocated by kzalloc in function tcf_em_tree_validate(), >> so cm_old is always NULL. >> >> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > Applied, 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 > -- 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
From: Oliver Hartkopp <socketcan@hartkopp.net> Date: Sun, 22 Jun 2014 18:27:34 +0200 > This is right for the first set of filters that is ever configured in > em_canid_change(). > > But what happens in an update case? > Does is turn the cm_old reference to a memory leak then? > > Or do we always get a brand new "struct tcf_ematch *m" here?? There is only one call to this method is tcf_em_validate(), which also unconditionally assigns new pointers to em->data without checking if an existing non-NULL value is in em->data. Specifically in the code path where em->ops->change is not implemented. -- 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/em_canid.c b/net/sched/em_canid.c index bfd34e4..7c292d4 100644 --- a/net/sched/em_canid.c +++ b/net/sched/em_canid.c @@ -125,7 +125,6 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len, { struct can_filter *conf = data; /* Array with rules */ struct canid_match *cm; - struct canid_match *cm_old = (struct canid_match *)m->data; int i; if (!len) @@ -181,12 +180,6 @@ static int em_canid_change(struct tcf_proto *tp, void *data, int len, m->datalen = sizeof(struct canid_match) + len; m->data = (unsigned long)cm; - - if (cm_old != NULL) { - pr_err("canid: Configuring an existing ematch!\n"); - kfree(cm_old); - } - return 0; }
tcf_ematch is allocated by kzalloc in function tcf_em_tree_validate(), so cm_old is always NULL. Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> --- net/sched/em_canid.c | 7 ------- 1 file changed, 7 deletions(-)