* net: Initialize return value in gro_cells_receive
@ 2020-10-06 18:53 Gregory Rose
2020-10-07 8:21 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Gregory Rose @ 2020-10-06 18:53 UTC (permalink / raw)
To: Netdev
The 'res' return value is uninitalized and may be returned with
some random value. Initialize to NET_RX_DROP as the default
return value.
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
index e095fb871d91..4e835960db07 100644
--- a/net/core/gro_cells.c
+++ b/net/core/gro_cells.c
@@ -13,7 +13,7 @@ int gro_cells_receive(struct gro_cells *gcells, struct
sk_buff *skb)
{
struct net_device *dev = skb->dev;
struct gro_cell *cell;
- int res;
+ int res = NET_RX_DROP;
rcu_read_lock();
if (unlikely(!(dev->flags & IFF_UP)))
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: net: Initialize return value in gro_cells_receive
2020-10-06 18:53 net: Initialize return value in gro_cells_receive Gregory Rose
@ 2020-10-07 8:21 ` Eric Dumazet
2020-10-07 15:50 ` Gregory Rose
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2020-10-07 8:21 UTC (permalink / raw)
To: Gregory Rose, Netdev
On 10/6/20 8:53 PM, Gregory Rose wrote:
> The 'res' return value is uninitalized and may be returned with
> some random value. Initialize to NET_RX_DROP as the default
> return value.
>
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>
> diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
> index e095fb871d91..4e835960db07 100644
> --- a/net/core/gro_cells.c
> +++ b/net/core/gro_cells.c
> @@ -13,7 +13,7 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
> {
> struct net_device *dev = skb->dev;
> struct gro_cell *cell;
> - int res;
> + int res = NET_RX_DROP;
>
> rcu_read_lock();
> if (unlikely(!(dev->flags & IFF_UP)))
I do not think this is needed.
Also, when/if sending a patch fixing a bug, we require a Fixes: tag.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: net: Initialize return value in gro_cells_receive
2020-10-07 8:21 ` Eric Dumazet
@ 2020-10-07 15:50 ` Gregory Rose
2020-10-07 16:37 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Gregory Rose @ 2020-10-07 15:50 UTC (permalink / raw)
To: Eric Dumazet, Netdev
On 10/7/2020 1:21 AM, Eric Dumazet wrote:
>
>
> On 10/6/20 8:53 PM, Gregory Rose wrote:
>> The 'res' return value is uninitalized and may be returned with
>> some random value. Initialize to NET_RX_DROP as the default
>> return value.
>>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>>
>> diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
>> index e095fb871d91..4e835960db07 100644
>> --- a/net/core/gro_cells.c
>> +++ b/net/core/gro_cells.c
>> @@ -13,7 +13,7 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
>> {
>> struct net_device *dev = skb->dev;
>> struct gro_cell *cell;
>> - int res;
>> + int res = NET_RX_DROP;
>>
>> rcu_read_lock();
>> if (unlikely(!(dev->flags & IFF_UP)))
>
> I do not think this is needed.
>
> Also, when/if sending a patch fixing a bug, we require a Fixes: tag.
>
> Thanks.
>
If it's not needed then feel free to ignore it. It just looked like
the unlikely case returns without setting the return value.
Thanks
- Greg
Thanks,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: net: Initialize return value in gro_cells_receive
2020-10-07 15:50 ` Gregory Rose
@ 2020-10-07 16:37 ` Eric Dumazet
2020-10-07 18:39 ` Gregory Rose
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2020-10-07 16:37 UTC (permalink / raw)
To: Gregory Rose, Eric Dumazet, Netdev
On 10/7/20 5:50 PM, Gregory Rose wrote:
>
>
> On 10/7/2020 1:21 AM, Eric Dumazet wrote:
>>
>>
>> On 10/6/20 8:53 PM, Gregory Rose wrote:
>>> The 'res' return value is uninitalized and may be returned with
>>> some random value. Initialize to NET_RX_DROP as the default
>>> return value.
>>>
>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>>>
>>> diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
>>> index e095fb871d91..4e835960db07 100644
>>> --- a/net/core/gro_cells.c
>>> +++ b/net/core/gro_cells.c
>>> @@ -13,7 +13,7 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
>>> {
>>> struct net_device *dev = skb->dev;
>>> struct gro_cell *cell;
>>> - int res;
>>> + int res = NET_RX_DROP;
>>>
>>> rcu_read_lock();
>>> if (unlikely(!(dev->flags & IFF_UP)))
>>
>> I do not think this is needed.
>>
>> Also, when/if sending a patch fixing a bug, we require a Fixes: tag.
>>
>> Thanks.
>>
> If it's not needed then feel free to ignore it. It just looked like
> the unlikely case returns without setting the return value.
Can you elaborate ? I do not see this problem in current upstream code.
If a compiler gave you a warning, please give its version, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: net: Initialize return value in gro_cells_receive
2020-10-07 16:37 ` Eric Dumazet
@ 2020-10-07 18:39 ` Gregory Rose
0 siblings, 0 replies; 5+ messages in thread
From: Gregory Rose @ 2020-10-07 18:39 UTC (permalink / raw)
To: Eric Dumazet, Netdev
On 10/7/2020 9:37 AM, Eric Dumazet wrote:
>
>
> On 10/7/20 5:50 PM, Gregory Rose wrote:
>>
>>
>> On 10/7/2020 1:21 AM, Eric Dumazet wrote:
>>>
>>>
>>> On 10/6/20 8:53 PM, Gregory Rose wrote:
>>>> The 'res' return value is uninitalized and may be returned with
>>>> some random value. Initialize to NET_RX_DROP as the default
>>>> return value.
>>>>
>>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>>>>
>>>> diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
>>>> index e095fb871d91..4e835960db07 100644
>>>> --- a/net/core/gro_cells.c
>>>> +++ b/net/core/gro_cells.c
>>>> @@ -13,7 +13,7 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
>>>> {
>>>> struct net_device *dev = skb->dev;
>>>> struct gro_cell *cell;
>>>> - int res;
>>>> + int res = NET_RX_DROP;
>>>>
>>>> rcu_read_lock();
>>>> if (unlikely(!(dev->flags & IFF_UP)))
>>>
>>> I do not think this is needed.
>>>
>>> Also, when/if sending a patch fixing a bug, we require a Fixes: tag.
>>>
>>> Thanks.
>>>
>> If it's not needed then feel free to ignore it. It just looked like
>> the unlikely case returns without setting the return value.
>
> Can you elaborate ? I do not see this problem in current upstream code.
>
> If a compiler gave you a warning, please give its version, thanks.
>
No, it's my misreading of the code - it jumps to the drop that is in the
middle of an if statement, sets res to NET_RX_DROP there and then jumps
to the unlock label.
My apologies.
- Greg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-07 18:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 18:53 net: Initialize return value in gro_cells_receive Gregory Rose
2020-10-07 8:21 ` Eric Dumazet
2020-10-07 15:50 ` Gregory Rose
2020-10-07 16:37 ` Eric Dumazet
2020-10-07 18:39 ` Gregory Rose
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).