Skip to content

ax25: Fix refcount imbalance on inbound connections #6213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

larsks
Copy link
Contributor

@larsks larsks commented Jun 8, 2024

This is a backport of 3c34fb0bd4a4237592c5ecb5b2e2531900c55774, which recently merged in the mainline kernel.

Without this patch, making ax.25 connections to a Pi will cause it to crash and possibly fail to reboot (unless you have the hardware watchdog enabled).

The original text of the commit is below.


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: 9fd75b6 ("ax25: Fix refcount leaks caused by ax25_cb_del()")
Signed-off-by: Lars Kellogg-Stedman [email protected]
Tested-by: Duoming Zhou [email protected]
Tested-by: Dan Cross [email protected]
Tested-by: Chris Maness [email protected]
Reviewed-by: Dan Carpenter [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski [email protected]
(cherry picked from commit 3c34fb0)

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: 9fd75b6 ("ax25: Fix refcount leaks caused by ax25_cb_del()")
Signed-off-by: Lars Kellogg-Stedman <[email protected]>
Tested-by: Duoming Zhou <[email protected]>
Tested-by: Dan Cross <[email protected]>
Tested-by: Chris Maness <[email protected]>
Reviewed-by: Dan Carpenter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit 3c34fb0)
@pelwell
Copy link
Contributor

pelwell commented Jun 17, 2024

Applied as 333b34f.

@pelwell pelwell closed this Jun 17, 2024
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jun 18, 2024
kernel: configs: Enable BLK_DEV_RBD
See: raspberrypi/linux#6226

kernel: Beefier command queue recovery
See: raspberrypi/linux#6221

kernel: ax25: Fix refcount imbalance on inbound connections
See: raspberrypi/linux#6213

kernel: Add SunFounder PiPower 3 dtoverlay
See: raspberrypi/linux#6227

kernel: Follow pwm_apply_might_sleep() rename in gpio-pwm and ir-rx51
See: raspberrypi/linux#6223
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Jun 18, 2024
kernel: configs: Enable BLK_DEV_RBD
See: raspberrypi/linux#6226

kernel: Beefier command queue recovery
See: raspberrypi/linux#6221

kernel: ax25: Fix refcount imbalance on inbound connections
See: raspberrypi/linux#6213

kernel: Add SunFounder PiPower 3 dtoverlay
See: raspberrypi/linux#6227

kernel: Follow pwm_apply_might_sleep() rename in gpio-pwm and ir-rx51
See: raspberrypi/linux#6223
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants