Commit 3c34fb0b authored by Lars Kellogg-Stedman's avatar Lars Kellogg-Stedman Committed by Jakub Kicinski

ax25: Fix refcount imbalance on inbound connections

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: 9fd75b66 ("ax25: Fix refcount leaks caused by ax25_cb_del()")
Signed-off-by: default avatarLars Kellogg-Stedman <lars@oddbit.com>
Tested-by: default avatarDuoming Zhou <duoming@zju.edu.cn>
Tested-by: default avatarDan Cross <crossd@gmail.com>
Tested-by: default avatarChris Maness <christopher.maness@gmail.com>
Reviewed-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/r/20240529210242.3346844-2-lars@oddbit.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 45c0a209
...@@ -1378,8 +1378,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock, ...@@ -1378,8 +1378,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock,
{ {
struct sk_buff *skb; struct sk_buff *skb;
struct sock *newsk; struct sock *newsk;
ax25_dev *ax25_dev;
DEFINE_WAIT(wait); DEFINE_WAIT(wait);
struct sock *sk; struct sock *sk;
ax25_cb *ax25;
int err = 0; int err = 0;
if (sock->state != SS_UNCONNECTED) if (sock->state != SS_UNCONNECTED)
...@@ -1434,6 +1436,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock, ...@@ -1434,6 +1436,10 @@ static int ax25_accept(struct socket *sock, struct socket *newsock,
kfree_skb(skb); kfree_skb(skb);
sk_acceptq_removed(sk); sk_acceptq_removed(sk);
newsock->state = SS_CONNECTED; 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: out:
release_sock(sk); release_sock(sk);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment