[PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[Jia-Ju Bai]

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;
 		goto fail_rx_pkt;
+	}
 
 	for (i = 0; i < n_scat_pkt; i++) {
 		int pad_len;
-- 
2.17.1

Re: [PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[Leon Romanovsky]

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
>

Re: [PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[Jia-Ju Bai]

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.
>

Re: [PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[Heiner Kallweit]

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.
>>

Re: [PATCH] ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle()

[Leon Romanovsky]

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