| Message ID | 20240529210242.3346844-2-lars@oddbit.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3c34fb0bd4a4237592c5ecb5b2e2531900c55774 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v5] ax25: Fix refcount imbalance on inbound connections | expand |
On Wed, May 29, 2024 at 05:02:43PM -0400, lars@oddbit.com wrote: > From: Lars Kellogg-Stedman <lars@oddbit.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@oddbit.com> > Tested-by: Duoming Zhou <duoming@zju.edu.cn> > Tested-by: Dan Cross <crossd@gmail.com> > Tested-by: Chris Maness <christopher.maness@gmail.com> > --- Thanks! Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> regards, dan carpenter
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 29 May 2024 17:02:43 -0400 you wrote: > From: Lars Kellogg-Stedman <lars@oddbit.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. > > [...] Here is the summary with links: - [v5] ax25: Fix refcount imbalance on inbound connections https://git.kernel.org/netdev/net/c/3c34fb0bd4a4 You are awesome, thank you!
Is this the only patch to get the last stable branch off of the
mainline (6.9) up to date?
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 8077cf2ee4480..d6f9fae06a9d8 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1378,8 +1378,10 @@ static int ax25_accept(struct socket *sock,
struct socket *newsock,
{
struct sk_buff *skb;
struct sock *newsk;
+ ax25_dev *ax25_dev;
DEFINE_WAIT(wait);
struct sock *sk;
+ ax25_cb *ax25;
int err = 0;
if (sock->state != SS_UNCONNECTED)
@@ -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);
#########################################
and I am going to guess that the next stable fork will have this
commit already applied?
Thanks in advance,
Chris KQ6UP
On Sat, Jun 1, 2024 at 4:10 PM Chris Maness
<christopher.maness@gmail.com> wrote:
>
> Awesome!
>
> Thanks,
> Chris Maness
> -Sent from my iPhone
>
>
> On Sat, Jun 1, 2024 at 4:00 PM <patchwork-bot+netdevbpf@kernel.org> wrote:
>>
>> Hello:
>>
>> This patch was applied to netdev/net.git (main)
>> by Jakub Kicinski <kuba@kernel.org>:
>>
>> On Wed, 29 May 2024 17:02:43 -0400 you wrote:
>> > From: Lars Kellogg-Stedman <lars@oddbit.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.
>> >
>> > [...]
>>
>> Here is the summary with links:
>> - [v5] ax25: Fix refcount imbalance on inbound connections
>> https://git.kernel.org/netdev/net/c/3c34fb0bd4a4
>>
>> You are awesome, thank you!
>> --
>> Deet-doot-dot, I am a bot.
>> https://korg.docs.kernel.org/patchwork/pwbot.html
>>
>>
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index 8077cf2ee44..d6f9fae06a9 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1378,8 +1378,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock, { struct sk_buff *skb; struct sock *newsk; + ax25_dev *ax25_dev; DEFINE_WAIT(wait); struct sock *sk; + ax25_cb *ax25; int err = 0; if (sock->state != SS_UNCONNECTED) @@ -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);