diff mbox series

ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

Message ID 20210307090757.22617-1-baijiaju1990@gmail.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle() | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jia-Ju Bai March 7, 2021, 9:07 a.m. UTC
When hif_scatter_req_get() returns NULL to scat_req, no error return
code of ath6kl_htc_rx_bundle() is assigned.
To fix this bug, status is assigned with -EINVAL in this case.

Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 drivers/net/wireless/ath/ath6kl/htc_mbox.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Leon Romanovsky March 7, 2021, 9:18 a.m. UTC | #1
On Sun, Mar 07, 2021 at 01:07:57AM -0800, Jia-Ju Bai wrote:
> When hif_scatter_req_get() returns NULL to scat_req, no error return
> code of ath6kl_htc_rx_bundle() is assigned.
> To fix this bug, status is assigned with -EINVAL in this case.
>
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
>  drivers/net/wireless/ath/ath6kl/htc_mbox.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/htc_mbox.c b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
> index 998947ef63b6..3f8857d19a0c 100644
> --- a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
> +++ b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
> @@ -1944,8 +1944,10 @@ static int ath6kl_htc_rx_bundle(struct htc_target *target,
>
>  	scat_req = hif_scatter_req_get(target->dev->ar);
>
> -	if (scat_req == NULL)
> +	if (scat_req == NULL) {
> +		status = -EINVAL;

I'm not sure about it.

David. Jakub,
Please be warned that patches from this guy are not so great.
I looked on 4 patches and 3 of them were wrong (2 in RDMA and 1 for mlx5)
plus this patch most likely is incorrect too.

Thanks

>  		goto fail_rx_pkt;
> +	}
>
>  	for (i = 0; i < n_scat_pkt; i++) {
>  		int pad_len;
> --
> 2.17.1
>
Jia-Ju Bai March 7, 2021, 9:31 a.m. UTC | #2
Hi Leon,

I am quite sorry for my incorrect patches...
My static analysis tool reports some possible bugs about error handling 
code, and thus I write some patches for the bugs that seem to be true in 
my opinion.
Because I am not familiar with many device drivers, some of my reported 
bugs can be false positives...


Best wishes,
Jia-Ju Bai

On 2021/3/7 17:18, Leon Romanovsky wrote:
> On Sun, Mar 07, 2021 at 01:07:57AM -0800, Jia-Ju Bai wrote:
>> When hif_scatter_req_get() returns NULL to scat_req, no error return
>> code of ath6kl_htc_rx_bundle() is assigned.
>> To fix this bug, status is assigned with -EINVAL in this case.
>>
>> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>>   drivers/net/wireless/ath/ath6kl/htc_mbox.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath6kl/htc_mbox.c b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>> index 998947ef63b6..3f8857d19a0c 100644
>> --- a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>> +++ b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>> @@ -1944,8 +1944,10 @@ static int ath6kl_htc_rx_bundle(struct htc_target *target,
>>
>>   	scat_req = hif_scatter_req_get(target->dev->ar);
>>
>> -	if (scat_req == NULL)
>> +	if (scat_req == NULL) {
>> +		status = -EINVAL;
> I'm not sure about it.
>
> David. Jakub,
> Please be warned that patches from this guy are not so great.
> I looked on 4 patches and 3 of them were wrong (2 in RDMA and 1 for mlx5)
> plus this patch most likely is incorrect too.
>
Heiner Kallweit March 7, 2021, 10:36 a.m. UTC | #3
On 07.03.2021 10:31, Jia-Ju Bai wrote:
> Hi Leon,
> 
> I am quite sorry for my incorrect patches...
> My static analysis tool reports some possible bugs about error handling code, and thus I write some patches for the bugs that seem to be true in my opinion.
> Because I am not familiar with many device drivers, some of my reported bugs can be false positives...

Then, before posting a patch for a driver, get familiar with it to
an extent that you can identify false positives. Relying on others
to detect the false positives is not the best approach.

> 
> 
> Best wishes,
> Jia-Ju Bai
> 
> On 2021/3/7 17:18, Leon Romanovsky wrote:
>> On Sun, Mar 07, 2021 at 01:07:57AM -0800, Jia-Ju Bai wrote:
>>> When hif_scatter_req_get() returns NULL to scat_req, no error return
>>> code of ath6kl_htc_rx_bundle() is assigned.
>>> To fix this bug, status is assigned with -EINVAL in this case.
>>>
>>> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>> ---
>>>   drivers/net/wireless/ath/ath6kl/htc_mbox.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath6kl/htc_mbox.c b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>>> index 998947ef63b6..3f8857d19a0c 100644
>>> --- a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>>> +++ b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
>>> @@ -1944,8 +1944,10 @@ static int ath6kl_htc_rx_bundle(struct htc_target *target,
>>>
>>>       scat_req = hif_scatter_req_get(target->dev->ar);
>>>
>>> -    if (scat_req == NULL)
>>> +    if (scat_req == NULL) {
>>> +        status = -EINVAL;
>> I'm not sure about it.
>>
>> David. Jakub,
>> Please be warned that patches from this guy are not so great.
>> I looked on 4 patches and 3 of them were wrong (2 in RDMA and 1 for mlx5)
>> plus this patch most likely is incorrect too.
>>
Leon Romanovsky March 7, 2021, 12:28 p.m. UTC | #4
On Sun, Mar 07, 2021 at 05:31:01PM +0800, Jia-Ju Bai wrote:
> Hi Leon,
>
> I am quite sorry for my incorrect patches...
> My static analysis tool reports some possible bugs about error handling
> code, and thus I write some patches for the bugs that seem to be true in my
> opinion.
> Because I am not familiar with many device drivers, some of my reported bugs
> can be false positives...

It will be much helpful if instead of writing new static analysis tool,
you will invest time and improve existing well known tools, like smatch
and sparse.

Right now, you didn't report bugs, but sent bunch of patches that most
of the time are incorrect. So it is not "some of my reported bugs can
be false positives...", but "some of my patches can fix real bugs by
chance".

Thanks
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath6kl/htc_mbox.c b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
index 998947ef63b6..3f8857d19a0c 100644
--- a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
+++ b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
@@ -1944,8 +1944,10 @@  static int ath6kl_htc_rx_bundle(struct htc_target *target,
 
 	scat_req = hif_scatter_req_get(target->dev->ar);
 
-	if (scat_req == NULL)
+	if (scat_req == NULL) {
+		status = -EINVAL;
 		goto fail_rx_pkt;
+	}
 
 	for (i = 0; i < n_scat_pkt; i++) {
 		int pad_len;