netdev - Re: [PATCH v5] ax25: Fix refcount imbalance on inbound connections

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: <babd0f3a-a77a-4d6a-971b-30835bf0e2ff@moroto.mountain>
Date: Thu, 30 May 2024 08:19:16 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: lars@...bit.com
Cc: netdev@...r.kernel.org, linux-hams@...r.kernel.org,
	Duoming Zhou <duoming@....edu.cn>, Dan Cross <crossd@...il.com>,
	Chris Maness <christopher.maness@...il.com>
Subject: Re: [PATCH v5] ax25: Fix refcount imbalance on inbound connections

On Wed, May 29, 2024 at 05:02:43PM -0400, lars@...bit.com wrote:
> From: Lars Kellogg-Stedman <lars@...bit.com>
> 
> When releasing a socket in ax25_release(), we call netdev_put() to
> decrease the refcount on the associated ax.25 device. However, the
> execution path for accepting an incoming connection never calls
> netdev_hold(). This imbalance leads to refcount errors, and ultimately
> to kernel crashes.
> 
> A typical call trace for the above situation will start with one of the
> following errors:
> 
>     refcount_t: decrement hit 0; leaking memory.
>     refcount_t: underflow; use-after-free.
> 
> And will then have a trace like:
> 
>     Call Trace:
>     <TASK>
>     ? show_regs+0x64/0x70
>     ? __warn+0x83/0x120
>     ? refcount_warn_saturate+0xb2/0x100
>     ? report_bug+0x158/0x190
>     ? prb_read_valid+0x20/0x30
>     ? handle_bug+0x3e/0x70
>     ? exc_invalid_op+0x1c/0x70
>     ? asm_exc_invalid_op+0x1f/0x30
>     ? refcount_warn_saturate+0xb2/0x100
>     ? refcount_warn_saturate+0xb2/0x100
>     ax25_release+0x2ad/0x360
>     __sock_release+0x35/0xa0
>     sock_close+0x19/0x20
>     [...]
> 
> On reboot (or any attempt to remove the interface), the kernel gets
> stuck in an infinite loop:
> 
>     unregister_netdevice: waiting for ax0 to become free. Usage count = 0
> 
> This patch corrects these issues by ensuring that we call netdev_hold()
> and ax25_dev_hold() for new connections in ax25_accept(). This makes the
> logic leading to ax25_accept() match the logic for ax25_bind(): in both
> cases we increment the refcount, which is ultimately decremented in
> ax25_release().
> 
> Fixes: 9fd75b66b8f6 ("ax25: Fix refcount leaks caused by ax25_cb_del()")
> Signed-off-by: Lars Kellogg-Stedman <lars@...bit.com>
> Tested-by: Duoming Zhou <duoming@....edu.cn>
> Tested-by: Dan Cross <crossd@...il.com>
> Tested-by: Chris Maness <christopher.maness@...il.com>
> ---

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@...aro.org>

regards,
dan carpenter