netdev - Re: selftests: net: pmtu.sh: Unable to handle kernel paging request at virtual address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iJBUAC6-tKhU0C95+rB4fk_FJD2B_sQRbKnGAJ3KkG+FA@mail.gmail.com>
Date: Tue, 8 Oct 2024 14:47:00 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Xin Long <lucien.xin@...il.com>
Cc: Naresh Kamboju <naresh.kamboju@...aro.org>, Hillf Danton <hdanton@...a.com>, 
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>, Netdev <netdev@...r.kernel.org>, 
	"Paul E. McKenney" <paulmck@...nel.org>, Linus Torvalds <torvalds@...ux-foundation.org>, 
	LKML <linux-kernel@...r.kernel.org>, Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: selftests: net: pmtu.sh: Unable to handle kernel paging request
 at virtual address

On Sun, Oct 6, 2024 at 8:08 PM Xin Long <lucien.xin@...il.com> wrote:
>
> Sorry for bringing up this issue, it recently occurred on my aarch64 kernel
> with blackhole_netdev backported. I tracked it down, and when deleting
> the netns, the path is:
>
> In cleanup_net():
>
>   default_device_exit_batch()
>     unregister_netdevice_many()
>       addrconf_ifdown() -> call_rcu(rcu, fib6_info_destroy_rcu) <--- [1]
>     netdev_run_todo()
>       rcu_barrier() <- [2]
>   ip6_route_net_exit() -> dst_entries_destroy(net->ip6_dst_ops) <--- [3]
>
> In fib6_info_destroy_rcu():
>
>   dst_dev_put()
>   dst_release() -> call_rcu(rcu, dst_destroy_rcu) <--- [5]
>
> In dst_destroy_rcu():
>   dst_destroy() -> dst_entries_add(dst->ops, -1); <--- [6]
>
> fib6_info_destroy_rcu() is scheduled at [1], rcu_barrier() will wait
> for fib6_info_destroy_rcu() to be done at [2]. However, another callback
> dst_destroy_rcu() is scheduled() in fib6_info_destroy_rcu() at [5], and
> there's no place calling rcu_barrier() to wait for dst_destroy_rcu() to
> be done. It means dst_entries_add() at [6] might be run later than
> dst_entries_destroy() at [3], then this UAF will trigger the panic.
>
> On Tue, Oct 17, 2023 at 1:02 PM Naresh Kamboju
> <naresh.kamboju@...aro.org> wrote:
> >
> > On Tue, 5 Sept 2023 at 17:55, Eric Dumazet <edumazet@...gle.com> wrote:
> > >
> > > On Tue, Sep 5, 2023 at 1:52 PM Hillf Danton <hdanton@...a.com> wrote:
> > > >
> > > > On Mon, 4 Sep 2023 13:29:57 +0200 Eric Dumazet <edumazet@...gle.com>
> > > > > On Sun, Sep 3, 2023 at 5:57=E2=80=AFAM Hillf Danton <hdanton@...a.com>
> > > > > > On Thu, 31 Aug 2023 15:12:30 +0200 Eric Dumazet <edumazet@...gle.com>
> > > > > > > --- a/net/core/dst.c
> > > > > > > +++ b/net/core/dst.c
> > > > > > > @@ -163,8 +163,13 @@ EXPORT_SYMBOL(dst_dev_put);
> > > > > > >
> > > > > > >  void dst_release(struct dst_entry *dst)
> > > > > > >  {
> > > > > > > -       if (dst && rcuref_put(&dst->__rcuref))
> > > > > > > +       if (dst && rcuref_put(&dst->__rcuref)) {
> > > > > > > +               if (!(dst->flags & DST_NOCOUNT)) {
> > > > > > > +                       dst->flags |= DST_NOCOUNT;
> > > > > > > +                       dst_entries_add(dst->ops, -1);
> > > > > >
> So I think it makes sense to NOT call dst_entries_add() in the path
> dst_destroy_rcu() -> dst_destroy(), as it does on the patch above,
> but I don't see it get posted.
>
> Hi, Eric, would you like to move forward with your patch above ?

I am planning to send it soon.

>
> Or we can also move the dst_entries_add(dst->ops, -1) from dst_destroy()
> to dst_release():

If we remove the code from dst_destroy(), we must do it from its two callers,
dst_release() and dst_release_immediate()

No big deal, I am adding a helper to make this a bit cleaner.


>
> Note, dst_destroy() is not used outside net/core/dst.c, we may delete
> EXPORT_SYMBOL(dst_destroy) in the future.

Which version are you looking at ?

Upstream got this already.

commit 03ba6dc035c60991033529e630bd1552b2bca4d7
Author: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Date:   Fri Feb 2 17:37:46 2024 +0100

    net: dst: Make dst_destroy() static and return void.