Received-SPF: pass (google.com: domain of linux-kernel+bounces-103172-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 14 Mar 2024 12:26:50 +0100
From: Florian Westphal <fw@strlen.de>
To: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>, xingwei lee <xrivendell7@gmail.com>,
	pabeni@redhat.com, davem@davemloft.net, kuba@kernel.org,
	linux-hams@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, ralf@linux-mips.org,
	syzkaller-bugs@googlegroups.com, samsun1006219@gmail.com
Subject: Re: KASAN: slab-use-after-free Read in ip_finish_output
Message-ID: <20240314112650.GE1038@breakpoint.cc>
References: <CABOYnLwtfAxS7WoMw-1_uxVe3EYajXRuzZfwaQEk0+7m6-B+ug@mail.gmail.com>
 <CANn89i+qLwyPLztPt6Mavjimyv0H_UihVVNfJXWLjcwrqOudTw@mail.gmail.com>
 <20240306103632.GC4420@breakpoint.cc>
 <CANn89iLe0KGjbSim5Qxxr6o0AjJVs7-h79UvMMXKOgGKQUosiA@mail.gmail.com>
 <20240312132107.GA1529@breakpoint.cc>
 <CANn89iLkDwnZdBY8CwkrQwCk2o7EAM9J1sv+uxU1tjKb=VB=Ag@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANn89iLkDwnZdBY8CwkrQwCk2o7EAM9J1sv+uxU1tjKb=VB=Ag@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)

Eric Dumazet <edumazet@google.com> wrote:
> Thanks for taking a look Florian.
> 
> Perhaps not messing with truesize at all would help ?
>
> Something based on this POC :
> 
>                 spin_lock(&qp->q.lock);
> +               if (!qp->q.sk) {
> +                       struct sock *sk = skb->sk;
> 
> +                       if (sk && refcount_inc_not_zero(&sk->sk_refcnt))
> +                               qp->q.sk = sk;

Unfortunetely I did not get this to work.

sk_refcnt is 0.  sk is kept alive by sock_wfree destructor.

I don't know how to recover from this, refcnt cannot be "repaired"
anymore.

I could artificially inflate sk_wmem counter by 1, to prevent release,
but that needs yet another sock_wfree-like destructor.

I'm already not sure how this existing scheme works, there are
multiple places that check for skb->destructor == sock_wfree,
yet we have is_skb_wmem helper that checks __sock_wfree and tcp_wfree.

Removing defrag from output seems like best option, but it will
surely break some scenarios.

Or, I could just fail reasm if sk refcount is already 0, that would
likely work too?