| Message ID | 20200601111201.64124-1-patrick.eigensatz@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipv4: nexthop: Fix deadcode issue by performing a proper NULL check | expand |
On 01/06/2020 14:12, patrickeigensatz@gmail.com wrote: > From: Patrick Eigensatz <patrickeigensatz@gmail.com> > > After allocating the spare nexthop group it should be tested for kzalloc() > returning NULL, instead the already used nexthop group (which cannot be > NULL at this point) had been tested so far. > > Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. > > Coverity-id: 1463885 > Reported-by: Coverity <scan-admin@coverity.com> > Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> > --- > net/ipv4/nexthop.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c > index 563f71bcb2d7..cb9412cd5e4b 100644 > --- a/net/ipv4/nexthop.c > +++ b/net/ipv4/nexthop.c > @@ -1118,10 +1118,10 @@ static struct nexthop *nexthop_create_group(struct net *net, > > /* spare group used for removals */ > nhg->spare = nexthop_grp_alloc(num_nh); > - if (!nhg) { > + if (!nhg->spare) { > kfree(nhg); > kfree(nh); > - return NULL; > + return ERR_PTR(-ENOMEM); > } > nhg->spare->spare = nhg; > > As Colin's similar patch[1] was rejected recently, this one also fixes the issue. This is targeted at -net. Fixes: 90f33bffa382 ("nexthops: don't modify published nexthop groups") Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Thanks! [1] https://lkml.org/lkml/2020/5/28/909
From: patrickeigensatz@gmail.com Date: Mon, 1 Jun 2020 13:12:01 +0200 > From: Patrick Eigensatz <patrickeigensatz@gmail.com> > > After allocating the spare nexthop group it should be tested for kzalloc() > returning NULL, instead the already used nexthop group (which cannot be > NULL at this point) had been tested so far. > > Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. > > Coverity-id: 1463885 > Reported-by: Coverity <scan-admin@coverity.com> > Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> Applied, thank you.
On 01/06/2020 21:06, David Miller wrote: > From: patrickeigensatz@gmail.com > Date: Mon, 1 Jun 2020 13:12:01 +0200 > >> From: Patrick Eigensatz <patrickeigensatz@gmail.com> >> >> After allocating the spare nexthop group it should be tested for kzalloc() >> returning NULL, instead the already used nexthop group (which cannot be >> NULL at this point) had been tested so far. >> >> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. >> >> Coverity-id: 1463885 >> Reported-by: Coverity <scan-admin@coverity.com> >> Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> > > Applied, thank you. > Hi Dave, I see this patch in -net-next but it should've been in -net as I wrote in my review[1]. This patch should go along with the recent nexthop set that fixes a few bugs, since it could result in a null ptr deref if the spare group cannot be allocated. How would you like to proceed? Should it be submitted for -net as well? Thanks, Nik [1] https://lkml.org/lkml/2020/6/1/391
On 02/06/2020 10:23, Nikolay Aleksandrov wrote: > On 01/06/2020 21:06, David Miller wrote: >> From: patrickeigensatz@gmail.com >> Date: Mon, 1 Jun 2020 13:12:01 +0200 >> >>> From: Patrick Eigensatz <patrickeigensatz@gmail.com> >>> >>> After allocating the spare nexthop group it should be tested for kzalloc() >>> returning NULL, instead the already used nexthop group (which cannot be >>> NULL at this point) had been tested so far. >>> >>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. >>> >>> Coverity-id: 1463885 >>> Reported-by: Coverity <scan-admin@coverity.com> >>> Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> >> >> Applied, thank you. >> > > Hi Dave, > I see this patch in -net-next but it should've been in -net as I wrote in my > review[1]. This patch should go along with the recent nexthop set that fixes > a few bugs, since it could result in a null ptr deref if the spare group cannot > be allocated. Obviously I forgot to mention in my review that it should go to -stable with the nexthop fix set. > How would you like to proceed? Should it be submitted for -net as well? > > Thanks, > Nik > > [1] https://lkml.org/lkml/2020/6/1/391 >
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Date: Tue, 2 Jun 2020 10:23:09 +0300 > On 01/06/2020 21:06, David Miller wrote: >> From: patrickeigensatz@gmail.com >> Date: Mon, 1 Jun 2020 13:12:01 +0200 >> >>> From: Patrick Eigensatz <patrickeigensatz@gmail.com> >>> >>> After allocating the spare nexthop group it should be tested for kzalloc() >>> returning NULL, instead the already used nexthop group (which cannot be >>> NULL at this point) had been tested so far. >>> >>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. >>> >>> Coverity-id: 1463885 >>> Reported-by: Coverity <scan-admin@coverity.com> >>> Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> >> >> Applied, thank you. >> > > Hi Dave, > I see this patch in -net-next but it should've been in -net as I wrote in my > review[1]. This patch should go along with the recent nexthop set that fixes > a few bugs, since it could result in a null ptr deref if the spare group cannot > be allocated. > How would you like to proceed? Should it be submitted for -net as well? When I'm leading up to the merge window I just toss everything into net-next and still queue things to -stable as needed.
On 03/06/2020 00:01, David Miller wrote: > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > Date: Tue, 2 Jun 2020 10:23:09 +0300 > >> On 01/06/2020 21:06, David Miller wrote: >>> From: patrickeigensatz@gmail.com >>> Date: Mon, 1 Jun 2020 13:12:01 +0200 >>> >>>> From: Patrick Eigensatz <patrickeigensatz@gmail.com> >>>> >>>> After allocating the spare nexthop group it should be tested for kzalloc() >>>> returning NULL, instead the already used nexthop group (which cannot be >>>> NULL at this point) had been tested so far. >>>> >>>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL. >>>> >>>> Coverity-id: 1463885 >>>> Reported-by: Coverity <scan-admin@coverity.com> >>>> Signed-off-by: Patrick Eigensatz <patrickeigensatz@gmail.com> >>> >>> Applied, thank you. >>> >> >> Hi Dave, >> I see this patch in -net-next but it should've been in -net as I wrote in my >> review[1]. This patch should go along with the recent nexthop set that fixes >> a few bugs, since it could result in a null ptr deref if the spare group cannot >> be allocated. >> How would you like to proceed? Should it be submitted for -net as well? > > When I'm leading up to the merge window I just toss everything into net-next > and still queue things to -stable as needed. > Got it, in that case could you please queue the patch for -stable? I checked https://patchwork.ozlabs.org/bundle/davem/stable/?state=* and https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git but didn't find it. Thanks, Nik
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c index 563f71bcb2d7..cb9412cd5e4b 100644 --- a/net/ipv4/nexthop.c +++ b/net/ipv4/nexthop.c @@ -1118,10 +1118,10 @@ static struct nexthop *nexthop_create_group(struct net *net, /* spare group used for removals */ nhg->spare = nexthop_grp_alloc(num_nh); - if (!nhg) { + if (!nhg->spare) { kfree(nhg); kfree(nh); - return NULL; + return ERR_PTR(-ENOMEM); } nhg->spare->spare = nhg;