netdev - Re: [PATCH v2] 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: <CANnsUMEyMqyNv-3gtqb60=KsDv1fxDska+QFNMzDtW0J7Pw48g@mail.gmail.com>
Date: Tue, 21 May 2024 07:32:14 -0700
From: Chris Maness <christopher.maness@...il.com>
To: Lars Kellogg-Stedman <lars@...bit.com>
Cc: netdev@...r.kernel.org, linux-hams@...r.kernel.org
Subject: Re: [PATCH v2] ax25: Fix refcount imbalance on inbound connections

Should I expect to this downstream in 6.9.2?

-Chris KQ6UP

On Tue, May 21, 2024 at 7:26 AM Lars Kellogg-Stedman <lars@...bit.com> wrote:
>
> The first version of this patch was posted only to the linux-hams
> mailing list. It has been difficult to get the patch reviewed, but the
> patch has now been tested successfully by three people (that includes
> me) who have all verified that it prevents the crashes that were
> previously plaguing inbound ax.25 connections.
>
> Related discussions:
>
> - https://marc.info/?l=linux-hams&m=171629285223248&w=2
> - https://marc.info/?l=linux-hams&m=171270115728031&w=2
>
> >8------------------------------------------------------8<
>
> 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 looks like this:
>
>     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(), balancing the
> calls to netdev_put() and ax25_dev_put() in ax25_release.
>
> Fixes: 7d8a3a477b
> Signed-off-by: Lars Kellogg-Stedman <lars@...bit.com>
> ---
>  net/ax25/af_ax25.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 8077cf2ee44..ff921272d40 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -1381,6 +1381,8 @@ static int ax25_accept(struct socket *sock, struct socket *newsock,
>         DEFINE_WAIT(wait);
>         struct sock *sk;
>         int err = 0;
> +       ax25_cb *ax25;
> +       ax25_dev *ax25_dev;
>
>         if (sock->state != SS_UNCONNECTED)
>                 return -EINVAL;
> @@ -1434,6 +1436,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock,
>         kfree_skb(skb);
>         sk_acceptq_removed(sk);
>         newsock->state = SS_CONNECTED;
> +       ax25 = sk_to_ax25(newsk);
> +       ax25_dev = ax25->ax25_dev;
> +       netdev_hold(ax25_dev->dev, &ax25->dev_tracker, GFP_ATOMIC);
> +       ax25_dev_hold(ax25_dev);
>
>  out:
>         release_sock(sk);
> --
> 2.45.1
>
> --
> Lars Kellogg-Stedman <lars@...bit.com> | larsks @ {irc,twitter,github}
> http://blog.oddbit.com/                | N1LKS
>


-- 
Thanks,
Chris Maness