From: Dan Carpenter <dan.carpenter@linaro.org>
To: Lars Kellogg-Stedman <lars@oddbit.com>,
Duoming Zhou <duoming@zju.edu.cn>
Cc: linux-hams@vger.kernel.org
Subject: Re: [PATCH RFC] ax25: Fix (more) netdev refcount issues
Date: Sat, 27 Apr 2024 11:48:48 +0300 [thread overview]
Message-ID: <472cb9d6-708e-4acd-b938-e12c3a19591e@moroto.mountain> (raw)
In-Reply-To: <6kant25vimoq36blyb5sjqgq3xxjcqbqeskbgf5zihdho3iulb@ni2lvhiytfrg>
The commit message needs a Fixes tag.
Fixes: d01ffb9eee4a ("ax25: add refcount in ax25_dev to avoid UAF bugs")
Let me add Duoming Zhou to the CC list. That commit is two years old
now. This sort of bug should have been caught by basic testing, right?
Could you confirm that that's actually the commit which breaks it?
regards,
dan carpenter
On Fri, Apr 26, 2024 at 05:29:40PM -0400, Lars Kellogg-Stedman wrote:
> Folks,
>
> I'm posting the following patch here before sending it to netdev in the
> hopes that someone can take a look at the change and comment on the
> correctness. This resolves all of the issues I've been experiencing
> recently with ax.25.
>
> ...
>
> When closing a socket, the ax.25 code releases references via
> netdev_put() and ax25_dev_put(). In the case when the socket was the
> result of an incoming connection, these references were never allocated in
> the first place, causing underflows in both ax25_dev->refcount and
> ax25_dev->dev->refcnt_tracker->untracked. This would result in a variety of
> errors:
>
> - After an initial connection and then again after several subsequent
> connections:
>
> refcount_t: decrement hit 0; leaking memory.
>
> - After several subsequent connections:
>
> refcount_t: underflow; use-after-free.
>
> A typical call trace for the above two issues would look 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, the kernel would get stuck in an infinite loop:
>
> unregister_netdevice: waiting for ax1 to become free. Usage count = 0
>
> The attached patch corrects all three of the above problems by ensuring
> that we call netdev_hold() and ax25_dev_hold() for incoming connections.
>
> Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com>
> ---
> net/ax25/ax25_in.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
> index 1cac25aca63..35a55ad05f2 100644
> --- a/net/ax25/ax25_in.c
> +++ b/net/ax25/ax25_in.c
> @@ -411,6 +411,8 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
> ax25->state = AX25_STATE_3;
>
> ax25_cb_add(ax25);
> + netdev_hold(ax25_dev->dev, &ax25->dev_tracker, GFP_ATOMIC);
> + ax25_dev_hold(ax25_dev);
>
> ax25_start_heartbeat(ax25);
> ax25_start_t3timer(ax25);
> --
> 2.44.0
>
> --
> Lars Kellogg-Stedman <lars@oddbit.com> | larsks @ {irc,twitter,github}
> http://blog.oddbit.com/ | N1LKS
next prev parent reply other threads:[~2024-04-27 8:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 21:29 [PATCH RFC] ax25: Fix (more) netdev refcount issues Lars Kellogg-Stedman
2024-04-27 8:48 ` Dan Carpenter [this message]
2024-04-27 17:15 ` Lars Kellogg-Stedman
2024-04-28 11:01 ` Dan Carpenter
2024-04-29 18:06 ` Lars Kellogg-Stedman
2024-04-30 10:08 ` Dan Carpenter
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=472cb9d6-708e-4acd-b938-e12c3a19591e@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=duoming@zju.edu.cn \
--cc=lars@oddbit.com \
--cc=linux-hams@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.