From: lars@oddbit.com
To: netdev@vger.kernel.org, linux-hams@vger.kernel.org
Cc: Lars Kellogg-Stedman <lars@oddbit.com>,
Duoming Zhou <duoming@zju.edu.cn>, Dan Cross <crossd@gmail.com>,
Chris Maness <christopher.maness@gmail.com>
Subject: [PATCH v5] ax25: Fix refcount imbalance on inbound connections
Date: Wed, 29 May 2024 17:02:43 -0400 [thread overview]
Message-ID: <20240529210242.3346844-2-lars@oddbit.com> (raw)
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>
---
net/ax25/af_ax25.c | 6 ++++++
1 file changed, 6 insertions(+)
v3:
- Address naveenm's comments regarding the ordering of variable declarations
(https://lore.kernel.org/netdev/SJ2PR18MB5635B7ADC7339BEDB79B183DA2EA2@SJ2PR18MB5635.namprd18.prod.outlook.com/)
v4:
- Respond to kuba's comments regarding the Fixes: tag
(https://lore.kernel.org/netdev/20240522100701.4d9edf99@kernel.org/)
v5:
- Respond to pabeni's comments regarding the Fixes: tag and running
checkpatch.pl
(https://lore.kernel.org/netdev/8e9a1c59f78a7774268bb6defed46df6f3771cbc.camel@redhat.com/)
- Respond to dan.carpenter's request about rewording the commit message
(https://lore.kernel.org/netdev/962afcda-8f67-400f-b3eb-951bf2e46fb7@moroto.mountain/)
- Accept duoming's suggestion for the Fixes: tag
(https://lore.kernel.org/netdev/3cf699c4.20d18.18fc4df304a.Coremail.duoming@zju.edu.cn/)
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);
--
2.45.1
next reply other threads:[~2024-05-29 21:21 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-29 21:02 lars [this message]
2024-05-30 5:19 ` [PATCH v5] ax25: Fix refcount imbalance on inbound connections Dan Carpenter
2024-06-01 23:00 ` patchwork-bot+netdevbpf
[not found] ` <CANnsUMG4sqomBWpU95u1q+guCAt9-G12cBCcvqsndkBWKn8zzA@mail.gmail.com>
2024-06-01 23:37 ` Chris Maness
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240529210242.3346844-2-lars@oddbit.com \
--to=lars@oddbit.com \
--cc=christopher.maness@gmail.com \
--cc=crossd@gmail.com \
--cc=duoming@zju.edu.cn \
--cc=linux-hams@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox