| Message ID | 20210307090757.22617-1-baijiaju1990@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath: ath6kl: fix error return code of ath6kl_htc_rx_bundle() | expand |
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 >
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. >
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. >>
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 --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;
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(-)