netdev - Re: [PATCH net] nfc: pn533: Wait for out_urb's completion in pn533_usb_send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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