In ath9k_wmi_cmd, the allocated network buffer needs to be released
if timeout happens. Otherwise memory will be leaked.
Signed-off-by: Navid Emamdoost <[email protected]>
---
drivers/net/wireless/ath/ath9k/wmi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index d1f6710ca63b..cdc146091194 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -336,6 +336,7 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
ath_dbg(common, WMI, "Timeout waiting for WMI command: %s\n",
wmi_cmd_to_name(cmd_id));
mutex_unlock(&wmi->op_mutex);
+ kfree_skb(skb);
return -ETIMEDOUT;
}
--
2.17.1
Navid Emamdoost <[email protected]> wrote:
> In ath9k_wmi_cmd, the allocated network buffer needs to be released
> if timeout happens. Otherwise memory will be leaked.
>
> Signed-off-by: Navid Emamdoost <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
728c1e2a05e4 ath9k: release allocated buffer if timed out
--
https://patchwork.kernel.org/patch/11135843/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Fri, Sep 6, 2019 at 11:59 AM Navid Emamdoost
<[email protected]> wrote:
>
> In ath9k_wmi_cmd, the allocated network buffer needs to be released
> if timeout happens. Otherwise memory will be leaked.
>
> Signed-off-by: Navid Emamdoost <[email protected]>
I wonder, did you actually test your patches? I ask, because it seems
that all your patches are of the same mechanical variety (produced by
some sort of research project?), and if I look around a bit, I see
several mistakes and regressions noted on your other patches. And
recently, I see someone reporting a 5.4 kernel regression, which looks
a lot like it was caused by this patch:
https://bugzilla.kernel.org/show_bug.cgi?id=207703#c1
I'll propose a revert, if there's no evidence this was actually tested
or otherwise confirmed to fix a real bug.
Brian
Hi Brian,
On Tue, May 12, 2020 at 11:57 AM Brian Norris <[email protected]> wrote:
>
> On Fri, Sep 6, 2019 at 11:59 AM Navid Emamdoost
> <[email protected]> wrote:
> >
> > In ath9k_wmi_cmd, the allocated network buffer needs to be released
> > if timeout happens. Otherwise memory will be leaked.
> >
> > Signed-off-by: Navid Emamdoost <[email protected]>
>
> I wonder, did you actually test your patches? I ask, because it seems
> that all your patches are of the same mechanical variety (produced by
> some sort of research project?), and if I look around a bit, I see
I found this via static analysis and as a result, did had the inputs
to test it with (like the way fuzzing works).
It may be beneficial if you could point me to any testing
infrastructure that you use or are aware of for future cases.
> several mistakes and regressions noted on your other patches. And
> recently, I see someone reporting a 5.4 kernel regression, which looks
> a lot like it was caused by this patch:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=207703#c1
>
> I'll propose a revert, if there's no evidence this was actually tested
> or otherwise confirmed to fix a real bug.
>
> Brian
--
Navid.
On Tue, May 12, 2020 at 8:25 PM Navid Emamdoost
<[email protected]> wrote:
> I found this via static analysis and as a result, did had the inputs
> to test it with (like the way fuzzing works).
Fuzzing is dynamic analysis, so I'm not sure how that fits.
> It may be beneficial if you could point me to any testing
> infrastructure that you use or are aware of for future cases.
syzbot (a real fuzzer -- I believe it uses fake USB devices [1])
caught the error, apparently:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=ced21a4c726bdc60b1680c050a284b08803bc64c
so you might look at using that too.
Traditionally, "testing your patches" means having hardware that runs
the driver in question when patching said driver. That likely won't
scale for researchers, but then, perhaps it just means you need to be
more clear on how you caught the issue and how you did (or didn't)
test it, so it's easier to reconcile your claims with the testing done
by real users.
If you only did static analysis, then we can be more confident in
reverting. The fuzz-tested revert is an even nicer bonus.
Brian
[1] https://github.com/google/syzkaller/blob/master/docs/syzbot.md#usb-bugs
https://github.com/google/syzkaller/blob/master/docs/linux/external_fuzzing_usb.md
On Wed, May 13, 2020 at 12:02 PM Brian Norris <[email protected]> wrote:
>
> On Wed, May 13, 2020 at 12:05 AM Kalle Valo <[email protected]> wrote:
> > Actually it's already reverted in -next, nobody just realised that it's
> > a regression from commit 728c1e2a05e4:
> >
> > ced21a4c726b ath9k: Fix use-after-free Read in htc_connect_service
>
> Nice.
>
> > v5.8-rc1 should be the first release having the fix.
>
> So I guess we have to wait until 5.8-rc1 (when this lands in mainline)
> to send this manually to [email protected]?
For the record, there are more reports of this, if I'm reading them right:
https://bugzilla.kernel.org/show_bug.cgi?id=207797