Linux-WPAN Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] net: ieee802154: fix error return code of dgram_sendmsg()
@ 2021-03-08  9:31 Jia-Ju Bai
  2021-03-08 10:19 ` Heiner Kallweit
  0 siblings, 1 reply; 5+ messages in thread
From: Jia-Ju Bai @ 2021-03-08  9:31 UTC (permalink / raw)
  To: alex.aring, stefan, davem, kuba
  Cc: linux-wpan, netdev, linux-kernel, Jia-Ju Bai

When sock_alloc_send_skb() returns NULL to skb, no error return code of
dgram_sendmsg() is assigned.
To fix this bug, err is assigned with -ENOMEM in this case.

Fixes: 78f821b64826 ("ieee802154: socket: put handling into one file")
Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 net/ieee802154/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index a45a0401adc5..a750b37c7e73 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -642,8 +642,10 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	skb = sock_alloc_send_skb(sk, hlen + tlen + size,
 				  msg->msg_flags & MSG_DONTWAIT,
 				  &err);
-	if (!skb)
+	if (!skb) {
+		err = -ENOMEM;
 		goto out_dev;
+	}
 
 	skb_reserve(skb, hlen);
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: ieee802154: fix error return code of dgram_sendmsg()
  2021-03-08  9:31 [PATCH] net: ieee802154: fix error return code of dgram_sendmsg() Jia-Ju Bai
@ 2021-03-08 10:19 ` Heiner Kallweit
  2021-03-08 12:18   ` Jia-Ju Bai
  0 siblings, 1 reply; 5+ messages in thread
From: Heiner Kallweit @ 2021-03-08 10:19 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: linux-wpan, netdev, linux-kernel, alex.aring, davem, kuba, stefan

On 08.03.2021 10:31, Jia-Ju Bai wrote:
> When sock_alloc_send_skb() returns NULL to skb, no error return code of
> dgram_sendmsg() is assigned.
> To fix this bug, err is assigned with -ENOMEM in this case.
> 

Please stop sending such nonsense. Basically all such patches you
sent so far are false positives. You have to start thinking,
don't blindly trust your robot.
In the case here the err variable is populated by sock_alloc_send_skb().

> Fixes: 78f821b64826 ("ieee802154: socket: put handling into one file")
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
>  net/ieee802154/socket.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
> index a45a0401adc5..a750b37c7e73 100644
> --- a/net/ieee802154/socket.c
> +++ b/net/ieee802154/socket.c
> @@ -642,8 +642,10 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>  	skb = sock_alloc_send_skb(sk, hlen + tlen + size,
>  				  msg->msg_flags & MSG_DONTWAIT,
>  				  &err);
> -	if (!skb)
> +	if (!skb) {
> +		err = -ENOMEM;
>  		goto out_dev;
> +	}
>  
>  	skb_reserve(skb, hlen);
>  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: ieee802154: fix error return code of dgram_sendmsg()
  2021-03-08 10:19 ` Heiner Kallweit
@ 2021-03-08 12:18   ` Jia-Ju Bai
  2021-03-08 13:33     ` Heiner Kallweit
  0 siblings, 1 reply; 5+ messages in thread
From: Jia-Ju Bai @ 2021-03-08 12:18 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: linux-wpan, netdev, linux-kernel, alex.aring, davem, kuba, stefan



On 2021/3/8 18:19, Heiner Kallweit wrote:
> On 08.03.2021 10:31, Jia-Ju Bai wrote:
>> When sock_alloc_send_skb() returns NULL to skb, no error return code of
>> dgram_sendmsg() is assigned.
>> To fix this bug, err is assigned with -ENOMEM in this case.
>>
> Please stop sending such nonsense. Basically all such patches you
> sent so far are false positives. You have to start thinking,
> don't blindly trust your robot.
> In the case here the err variable is populated by sock_alloc_send_skb().

Ah, sorry, it is my fault :(
I did not notice that the err variable is populated by 
sock_alloc_send_skb().
I will think more carefully before sending patches.

By the way, I wonder how to report and discuss possible bugs that I am 
not quite sure of?
Some people told me that sending patches is better than reporting bugs 
via Bugzilla, so I write the patches of these possible bugs...
Do you have any advice?

Thanks a lot!


Best wishes,
Jia-Ju Bai
>
>> Fixes: 78f821b64826 ("ieee802154: socket: put handling into one file")
>> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>>   net/ieee802154/socket.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
>> index a45a0401adc5..a750b37c7e73 100644
>> --- a/net/ieee802154/socket.c
>> +++ b/net/ieee802154/socket.c
>> @@ -642,8 +642,10 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>>   	skb = sock_alloc_send_skb(sk, hlen + tlen + size,
>>   				  msg->msg_flags & MSG_DONTWAIT,
>>   				  &err);
>> -	if (!skb)
>> +	if (!skb) {
>> +		err = -ENOMEM;
>>   		goto out_dev;
>> +	}
>>   
>>   	skb_reserve(skb, hlen);
>>   
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: ieee802154: fix error return code of dgram_sendmsg()
  2021-03-08 12:18   ` Jia-Ju Bai
@ 2021-03-08 13:33     ` Heiner Kallweit
  2021-03-08 13:49       ` Jia-Ju Bai
  0 siblings, 1 reply; 5+ messages in thread
From: Heiner Kallweit @ 2021-03-08 13:33 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: linux-wpan, netdev, linux-kernel, alex.aring, davem, kuba, stefan

On 08.03.2021 13:18, Jia-Ju Bai wrote:
> 
> 
> On 2021/3/8 18:19, Heiner Kallweit wrote:
>> On 08.03.2021 10:31, Jia-Ju Bai wrote:
>>> When sock_alloc_send_skb() returns NULL to skb, no error return code of
>>> dgram_sendmsg() is assigned.
>>> To fix this bug, err is assigned with -ENOMEM in this case.
>>>
>> Please stop sending such nonsense. Basically all such patches you
>> sent so far are false positives. You have to start thinking,
>> don't blindly trust your robot.
>> In the case here the err variable is populated by sock_alloc_send_skb().
> 
> Ah, sorry, it is my fault :(
> I did not notice that the err variable is populated by sock_alloc_send_skb().
> I will think more carefully before sending patches.
> 
> By the way, I wonder how to report and discuss possible bugs that I am not quite sure of?
> Some people told me that sending patches is better than reporting bugs via Bugzilla, so I write the patches of these possible bugs...
> Do you have any advice?
> 

If you're quite sure that something is a bug then sending a patch is fine.
Your submissions more or less all being false positives shows that this
takes more than just forwarding bot findings, especially if you have no
idea yet regarding the quality of the bot.
Alternatively you can contact the maintainer and respective mailing list.
But again, maintainers typically are very busy and you should have done
all you can to analyze the suspected bug.

What I'd do being in your shoes:
Take the first 10 findings of a new bot and analyze in detail whether
findings are correct or false positives. Of course this means you
need to get familiar with the affected code in the respective driver.
If false positive ratio is > 5% I wouldn't send out patches w/o more
detailed analysis per finding.

Worst case a maintainer is busy and can't review your submission in time,
and the incorrect fix is applied and breaks the driver.
Typically this shouldn't happen however because Dave/Jakub won't apply
a patch w/o Ack from the respective maintainer.

Disclaimer:
I can only speak for myself. Other maintainers may see this differently.

> Thanks a lot!
> 
> 
> Best wishes,
> Jia-Ju Bai
>>
>>> Fixes: 78f821b64826 ("ieee802154: socket: put handling into one file")
>>> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>> ---
>>>   net/ieee802154/socket.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
>>> index a45a0401adc5..a750b37c7e73 100644
>>> --- a/net/ieee802154/socket.c
>>> +++ b/net/ieee802154/socket.c
>>> @@ -642,8 +642,10 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>>>       skb = sock_alloc_send_skb(sk, hlen + tlen + size,
>>>                     msg->msg_flags & MSG_DONTWAIT,
>>>                     &err);
>>> -    if (!skb)
>>> +    if (!skb) {
>>> +        err = -ENOMEM;
>>>           goto out_dev;
>>> +    }
>>>         skb_reserve(skb, hlen);
>>>  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: ieee802154: fix error return code of dgram_sendmsg()
  2021-03-08 13:33     ` Heiner Kallweit
@ 2021-03-08 13:49       ` Jia-Ju Bai
  0 siblings, 0 replies; 5+ messages in thread
From: Jia-Ju Bai @ 2021-03-08 13:49 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: linux-wpan, netdev, linux-kernel, alex.aring, davem, kuba, stefan



On 2021/3/8 21:33, Heiner Kallweit wrote:
> On 08.03.2021 13:18, Jia-Ju Bai wrote:
>>
>> On 2021/3/8 18:19, Heiner Kallweit wrote:
>>> On 08.03.2021 10:31, Jia-Ju Bai wrote:
>>>> When sock_alloc_send_skb() returns NULL to skb, no error return code of
>>>> dgram_sendmsg() is assigned.
>>>> To fix this bug, err is assigned with -ENOMEM in this case.
>>>>
>>> Please stop sending such nonsense. Basically all such patches you
>>> sent so far are false positives. You have to start thinking,
>>> don't blindly trust your robot.
>>> In the case here the err variable is populated by sock_alloc_send_skb().
>> Ah, sorry, it is my fault :(
>> I did not notice that the err variable is populated by sock_alloc_send_skb().
>> I will think more carefully before sending patches.
>>
>> By the way, I wonder how to report and discuss possible bugs that I am not quite sure of?
>> Some people told me that sending patches is better than reporting bugs via Bugzilla, so I write the patches of these possible bugs...
>> Do you have any advice?
>>
> If you're quite sure that something is a bug then sending a patch is fine.
> Your submissions more or less all being false positives shows that this
> takes more than just forwarding bot findings, especially if you have no
> idea yet regarding the quality of the bot.
> Alternatively you can contact the maintainer and respective mailing list.
> But again, maintainers typically are very busy and you should have done
> all you can to analyze the suspected bug.
>
> What I'd do being in your shoes:
> Take the first 10 findings of a new bot and analyze in detail whether
> findings are correct or false positives. Of course this means you
> need to get familiar with the affected code in the respective driver.
> If false positive ratio is > 5% I wouldn't send out patches w/o more
> detailed analysis per finding.
>
> Worst case a maintainer is busy and can't review your submission in time,
> and the incorrect fix is applied and breaks the driver.
> Typically this shouldn't happen however because Dave/Jakub won't apply
> a patch w/o Ack from the respective maintainer.
>
> Disclaimer:
> I can only speak for myself. Other maintainers may see this differently.

Okay, thanks a lot for the very helpful advice :)
I will carefully check the bug report and try my best to write correct 
patches.


Best wishes,
Jia-Ju Bai

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08  9:31 [PATCH] net: ieee802154: fix error return code of dgram_sendmsg() Jia-Ju Bai
2021-03-08 10:19 ` Heiner Kallweit
2021-03-08 12:18   ` Jia-Ju Bai
2021-03-08 13:33     ` Heiner Kallweit
2021-03-08 13:49       ` Jia-Ju Bai

Linux-WPAN Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wpan/0 linux-wpan/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wpan linux-wpan/ https://lore.kernel.org/linux-wpan \
		linux-wpan@vger.kernel.org
	public-inbox-index linux-wpan

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wpan


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git