linux-kernel - Re: [PATCH] drivers: hamradio: 6pack: fix UAF bug caused by mod

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20220215203955.7d7a3eed@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 15 Feb 2022 20:39:55 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Duoming Zhou <duoming@....edu.cn>
Cc:     linux-hams@...r.kernel.org, ajk@...nets.uni-bremen.de,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: hamradio: 6pack: fix UAF bug caused by
 mod_timer()

On Wed, 16 Feb 2022 10:35:49 +0800 Duoming Zhou wrote:
> Although del_timer_sync() in sixpack_close() waits for the timer handler
> to finish its execution and then releases the timer, the mod_timer()
> in sp_xmit_on_air() could be called by userspace syscall such as
> ax25_sendmsg(), ax25_connect() and ax25_ioctl() and wakes up the timer
> again. If the timer uses sp_xmit_on_air() to write data on pty work queue
> that already released by unregister_netdev(), the UAF bug will happen.

Do you mean sp->xbuff access? It's released right before the netdev
itself is freed. Checking dev->something is also a UAF.

> One of the possible race conditions is shown below:
> 
>       (USE)                     |      (FREE)
> ax25_sendmsg()                  |
>   ax25_queue_xmit()             |
>     ...                         |
>     sp_encaps()                 |  sixpack_close()
>       sp_xmit_on_air()          |    del_timer_sync(&sp->tx_t)
>         mod_timer(&sp->tx_t,..) |    ...
>         (wait a while)          |    unregister_netdev(sp->dev)) //FREE

Please clarify what is getting freed. 

>       sp_xmit_on_air()          |    ...
>         pty_write()             |
>           queue_work_on() //USE |
> 
> The corresponding fail log is shown below:
> ===============================================================
> BUG: KASAN: use-after-free in __run_timers.part.0+0x170/0x470
> Write of size 8 at addr ffff88800a652ab8 by task swapper/2/0
> ...
> Call Trace:
>   ...
>   queue_work_on+0x3f/0x50
>   pty_write+0xcd/0xe0pty_write+0xcd/0xe0
>   sp_xmit_on_air+0xb2/0x1f0
>   call_timer_fn+0x28/0x150
>   __run_timers.part.0+0x3c2/0x470
>   run_timer_softirq+0x3b/0x80
>   __do_softirq+0xf1/0x380
>   ...
> 
> This patch add condition check in sp_xmit_on_air(). If the
> registration status of net_device is not equal to NETREG_REGISTERED,
> the sp_xmit_on_air() will not write data to pty work queue and
> return instead.

I don't think this works as mentioned above. The question is why the tx
queue is not stopped.

> diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
> index b1fc153125d..7ee25e06915 100644
> --- a/drivers/net/hamradio/6pack.c
> +++ b/drivers/net/hamradio/6pack.c
> @@ -141,7 +141,8 @@ static void sp_xmit_on_air(struct timer_list *t)
>  	struct sixpack *sp = from_timer(sp, t, tx_t);
>  	int actual, when = sp->slottime;
>  	static unsigned char random;
> -
> +	if (sp->dev->reg_state !=  NETREG_REGISTERED)
> +		return;
>  	random = random * 17 + 41;
>  
>  	if (((sp->status1 & SIXP_DCD_MASK) == 0) && (random < sp->persistence)) {