* [bug report] net: Prepare gro for packet consuming gro callbacks
@ 2017-02-16 20:11 Dan Carpenter
2017-02-17 8:18 ` Steffen Klassert
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-02-16 20:11 UTC (permalink / raw)
To: kernel-janitors
Hello Steffen Klassert,
The patch 25393d3fc055: "net: Prepare gro for packet consuming gro
callbacks" from Feb 15, 2017, leads to the following Smatch
warning:
net/core/dev.c:4522 dev_gro_receive()
error: 'pp' dereferencing possible ERR_PTR()
net/core/dev.c
4510 if (&ptype->list = head)
4511 goto normal;
4512
4513 if (IS_ERR(pp) && PTR_ERR(pp) = -EINPROGRESS) {
Smatch sees this and assumes that it's possible for "pp" to be an error
pointer that's not -EINPROGRESS.
4514 ret = GRO_CONSUMED;
4515 goto ok;
4516 }
4517
4518 same_flow = NAPI_GRO_CB(skb)->same_flow;
4519 ret = NAPI_GRO_CB(skb)->free ? GRO_MERGED_FREE : GRO_MERGED;
4520
4521 if (pp) {
4522 struct sk_buff *nskb = *pp;
^^^
Leading to an oops.
4523
4524 *pp = nskb->next;
4525 nskb->next = NULL;
4526 napi_gro_complete(nskb);
4527 napi->gro_count--;
4528 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] net: Prepare gro for packet consuming gro callbacks
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
@ 2017-02-17 8:18 ` Steffen Klassert
2017-02-17 10:13 ` Dan Carpenter
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-02-17 8:18 UTC (permalink / raw)
To: kernel-janitors
Hi Dan.
On Thu, Feb 16, 2017 at 11:11:27PM +0300, Dan Carpenter wrote:
> Hello Steffen Klassert,
>
> The patch 25393d3fc055: "net: Prepare gro for packet consuming gro
> callbacks" from Feb 15, 2017, leads to the following Smatch
> warning:
>
> net/core/dev.c:4522 dev_gro_receive()
> error: 'pp' dereferencing possible ERR_PTR()
>
> net/core/dev.c
> 4510 if (&ptype->list = head)
> 4511 goto normal;
> 4512
> 4513 if (IS_ERR(pp) && PTR_ERR(pp) = -EINPROGRESS) {
>
>
> Smatch sees this and assumes that it's possible for "pp" to be an error
> pointer that's not -EINPROGRESS.
pp can be either a vaild pointer, NULL or -EINPROGRESS,
nothing else. So I guess this is ok.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] net: Prepare gro for packet consuming gro callbacks
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
2017-02-17 8:18 ` Steffen Klassert
@ 2017-02-17 10:13 ` Dan Carpenter
2017-02-17 10:22 ` Steffen Klassert
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-02-17 10:13 UTC (permalink / raw)
To: kernel-janitors
On Fri, Feb 17, 2017 at 09:18:25AM +0100, Steffen Klassert wrote:
> Hi Dan.
>
> On Thu, Feb 16, 2017 at 11:11:27PM +0300, Dan Carpenter wrote:
> > Hello Steffen Klassert,
> >
> > The patch 25393d3fc055: "net: Prepare gro for packet consuming gro
> > callbacks" from Feb 15, 2017, leads to the following Smatch
> > warning:
> >
> > net/core/dev.c:4522 dev_gro_receive()
> > error: 'pp' dereferencing possible ERR_PTR()
> >
> > net/core/dev.c
> > 4510 if (&ptype->list = head)
> > 4511 goto normal;
> > 4512
> > 4513 if (IS_ERR(pp) && PTR_ERR(pp) = -EINPROGRESS) {
> >
> >
> > Smatch sees this and assumes that it's possible for "pp" to be an error
> > pointer that's not -EINPROGRESS.
>
> pp can be either a vaild pointer, NULL or -EINPROGRESS,
> nothing else. So I guess this is ok.
Could you change it to:
if (PTR_ERR(pp) = -EINPROGRESS) {
That would silence the warning and make the code more clear I think.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] net: Prepare gro for packet consuming gro callbacks
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
2017-02-17 8:18 ` Steffen Klassert
2017-02-17 10:13 ` Dan Carpenter
@ 2017-02-17 10:22 ` Steffen Klassert
2017-02-17 10:39 ` Dan Carpenter
2017-02-17 13:39 ` Dan Carpenter
4 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2017-02-17 10:22 UTC (permalink / raw)
To: kernel-janitors
On Fri, Feb 17, 2017 at 01:13:05PM +0300, Dan Carpenter wrote:
> On Fri, Feb 17, 2017 at 09:18:25AM +0100, Steffen Klassert wrote:
> > Hi Dan.
> >
> > On Thu, Feb 16, 2017 at 11:11:27PM +0300, Dan Carpenter wrote:
> > > Hello Steffen Klassert,
> > >
> > > The patch 25393d3fc055: "net: Prepare gro for packet consuming gro
> > > callbacks" from Feb 15, 2017, leads to the following Smatch
> > > warning:
> > >
> > > net/core/dev.c:4522 dev_gro_receive()
> > > error: 'pp' dereferencing possible ERR_PTR()
> > >
> > > net/core/dev.c
> > > 4510 if (&ptype->list = head)
> > > 4511 goto normal;
> > > 4512
> > > 4513 if (IS_ERR(pp) && PTR_ERR(pp) = -EINPROGRESS) {
> > >
> > >
> > > Smatch sees this and assumes that it's possible for "pp" to be an error
> > > pointer that's not -EINPROGRESS.
> >
> > pp can be either a vaild pointer, NULL or -EINPROGRESS,
> > nothing else. So I guess this is ok.
>
> Could you change it to:
>
> if (PTR_ERR(pp) = -EINPROGRESS) {
>
> That would silence the warning and make the code more clear I think.
I had it like that, but some other code checking tool
complained that I check -EINPROGRESS without checking
if this is an error pointer. So I changed it to that
what I have now.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] net: Prepare gro for packet consuming gro callbacks
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
` (2 preceding siblings ...)
2017-02-17 10:22 ` Steffen Klassert
@ 2017-02-17 10:39 ` Dan Carpenter
2017-02-17 13:39 ` Dan Carpenter
4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-02-17 10:39 UTC (permalink / raw)
To: kernel-janitors
On Fri, Feb 17, 2017 at 11:22:41AM +0100, Steffen Klassert wrote:
> On Fri, Feb 17, 2017 at 01:13:05PM +0300, Dan Carpenter wrote:
> > On Fri, Feb 17, 2017 at 09:18:25AM +0100, Steffen Klassert wrote:
> > > Hi Dan.
> > >
> > > On Thu, Feb 16, 2017 at 11:11:27PM +0300, Dan Carpenter wrote:
> > > > Hello Steffen Klassert,
> > > >
> > > > The patch 25393d3fc055: "net: Prepare gro for packet consuming gro
> > > > callbacks" from Feb 15, 2017, leads to the following Smatch
> > > > warning:
> > > >
> > > > net/core/dev.c:4522 dev_gro_receive()
> > > > error: 'pp' dereferencing possible ERR_PTR()
> > > >
> > > > net/core/dev.c
> > > > 4510 if (&ptype->list = head)
> > > > 4511 goto normal;
> > > > 4512
> > > > 4513 if (IS_ERR(pp) && PTR_ERR(pp) = -EINPROGRESS) {
> > > >
> > > >
> > > > Smatch sees this and assumes that it's possible for "pp" to be an error
> > > > pointer that's not -EINPROGRESS.
> > >
> > > pp can be either a vaild pointer, NULL or -EINPROGRESS,
> > > nothing else. So I guess this is ok.
> >
> > Could you change it to:
> >
> > if (PTR_ERR(pp) = -EINPROGRESS) {
> >
> > That would silence the warning and make the code more clear I think.
>
> I had it like that, but some other code checking tool
> complained that I check -EINPROGRESS without checking
> if this is an error pointer. So I changed it to that
> what I have now.
>
Why would it complain about that???
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] net: Prepare gro for packet consuming gro callbacks
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
` (3 preceding siblings ...)
2017-02-17 10:39 ` Dan Carpenter
@ 2017-02-17 13:39 ` Dan Carpenter
4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-02-17 13:39 UTC (permalink / raw)
To: kernel-janitors
The only static checkers which care about ERR_PTR are Smatch and
Coccinelle. I don't think Coccinelle would care about that so it must
be a Smatch warning...
:/
The only thing I can think of is that this is a Smatch warning that
"pp is not an error pointer". And that would be because the cross
function database wasn't fully populated... I'm not positive on that.
Very strange.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-17 13:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-16 20:11 [bug report] net: Prepare gro for packet consuming gro callbacks Dan Carpenter
2017-02-17 8:18 ` Steffen Klassert
2017-02-17 10:13 ` Dan Carpenter
2017-02-17 10:22 ` Steffen Klassert
2017-02-17 10:39 ` Dan Carpenter
2017-02-17 13:39 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).