Message ID | 20190729102611.2338-1-baijiaju1990@gmail.com |
---|---|
State | Rejected |
Delegated to: | David Miller |
Headers | show |
Series | net: geneve: Fix a possible null-pointer dereference in geneve_link_config() | expand |
On Mon, 29 Jul 2019 18:26:11 +0800, Jia-Ju Bai wrote: > --- a/drivers/net/geneve.c > +++ b/drivers/net/geneve.c > @@ -1521,9 +1521,10 @@ static void geneve_link_config(struct net_device *dev, > rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0, > NULL, 0); > > - if (rt && rt->dst.dev) > + if (rt && rt->dst.dev) { > ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN; > - ip6_rt_put(rt); > + ip6_rt_put(rt); > + } > break; > } > #endif Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev being NULL? You'd leak the reference in such case. Jiri
On Mon, 29 Jul 2019 12:30:55 +0200, Jiri Benc wrote: > Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev > being NULL? You'd leak the reference in such case. In fact, you're introducing a bug, not fixing one. ip6_rt_put does accept NULL parameter. And it seems you already have been told that? Nacked-by: Jiri Benc <jbenc@redhat.com> Jiri
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c index cb2ea8facd8d..a47a1b31b166 100644 --- a/drivers/net/geneve.c +++ b/drivers/net/geneve.c @@ -1521,9 +1521,10 @@ static void geneve_link_config(struct net_device *dev, rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0, NULL, 0); - if (rt && rt->dst.dev) + if (rt && rt->dst.dev) { ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN; - ip6_rt_put(rt); + ip6_rt_put(rt); + } break; } #endif
In geneve_link_config(), there is an if statement on line 1524 to check whether rt is NULL: if (rt && rt->dst.dev) When rt is NULL, it is used on line 1526: ip6_rt_put(rt) dst_release(&rt->dst); Thus, a possible null-pointer dereference may occur. To fix this bug, ip6_rt_put(rt) is called when rt is not NULL. This bug is found by a static analysis tool STCheck written by us. Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- drivers/net/geneve.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)