[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230105212606.2f14e5a9@kernel.org>
Date: Thu, 5 Jan 2023 21:26:06 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Minsuk Kang <linuxlovemin@...sei.ac.kr>
Cc: krzysztof.kozlowski@...aro.org, netdev@...r.kernel.org,
aloisio.almeida@...nbossa.org, sameo@...ux.intel.com,
lauro.venancio@...nbossa.org, linville@...driver.com,
dokyungs@...sei.ac.kr, jisoo.jang@...sei.ac.kr
Subject: Re: [PATCH net] nfc: pn533: Wait for out_urb's completion in
pn533_usb_send_frame()
On Wed, 4 Jan 2023 21:17:11 +0900 Minsuk Kang wrote:
> Fix a use-after-free that occurs in hcd when in_urb sent from
> pn533_usb_send_frame() is completed earlier than out_urb. Its callback
> frees the skb data in pn533_send_async_complete() that is used as a
> transfer buffer of out_urb. Wait before sending in_urb until the
> callback of out_urb is called. To modify the callback of out_urb alone,
> separate the complete function of out_urb and ack_urb.
>
> Found by a modified version of syzkaller.
>
> BUG: KASAN: use-after-free in dummy_timer
> Call Trace:
> memcpy
> dummy_timer
> call_timer_fn
> run_timer_softirq
> __do_softirq
> irq_exit_rcu
> sysvec_apic_timer_interrupt
Could you add fine names and line numbers to the stack trace
(use scripts/decode_stacktrace.sh)? dummy_timer is a very generic
name, it'd be useful to tie it to the gadget device via file name.
> Fixes: c46ee38620a2 ("NFC: pn533: add NXP pn533 nfc device driver")
> Signed-off-by: Minsuk Kang <linuxlovemin@...sei.ac.kr>
> ---
> drivers/nfc/pn533/usb.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
> index 6f71ac72012e..325818fbaf3b 100644
> --- a/drivers/nfc/pn533/usb.c
> +++ b/drivers/nfc/pn533/usb.c
> @@ -48,6 +48,8 @@ struct pn533_usb_phy {
> struct usb_interface *interface;
>
> struct urb *out_urb;
> + struct completion *out_done;
Why keep this pointer for the lifetime of the device instead of putting
it on the stack? For inspiration please take a look at
struct pn533_acr122_poweron_rdr_arg
Powered by blists - more mailing lists