Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Wed, 25 Oct 2023 12:27:05 +0200
From:   Sabrina Dubroca <sd@queasysnail.net>
To:     Hangyu Hua <hbh25y@gmail.com>, kuba@kernel.org
Cc:     borisp@nvidia.com, john.fastabend@gmail.com, davem@davemloft.net,
        edumazet@google.com, pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: tls: Fix possible NULL-pointer dereference in
 tls_decrypt_device() and tls_decrypt_sw()
Message-ID: <ZTjteQgXWKXDqnos@hog>
References: <20231023080611.19244-1-hbh25y@gmail.com>
 <ZTZ9H4aDB45RzrFD@hog>
 <120e6c2c-6122-41db-8c46-7753e9659c70@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <120e6c2c-6122-41db-8c46-7753e9659c70@gmail.com>
Precedence: bulk

2023-10-24, 10:17:08 +0800, Hangyu Hua wrote:
> On 23/10/2023 22:03, Sabrina Dubroca wrote:
> > 2023-10-23, 16:06:11 +0800, Hangyu Hua wrote:
> > > tls_rx_one_record can be called in tls_sw_splice_read and tls_sw_read_sock
> > > with msg being NULL. This may lead to null pointer dereferences in
> > > tls_decrypt_device and tls_decrypt_sw.
> > > 
> > > Fix this by adding a check.
> > 
> > Have you actually hit this NULL dereference? I don't see how it can
> > happen.
> > 
> > darg->zc is 0 in both cases, so tls_decrypt_device doesn't call
> > skb_copy_datagram_msg.
> > 
> > tls_decrypt_sw will call tls_decrypt_sg with out_iov = &msg->msg_iter
> > (a bogus pointer but no NULL deref yet), and darg->zc is still
> > 0. tls_decrypt_sg skips the use of out_iov/out_sg and allocates
> > clear_skb, and the next place where it would use out_iov is skipped
> > because we have clear_skb.
> 
> My bad. I only checked &msg->msg_iter's address in tls_decrypt_sw and found
> it was wrong. Do I need to make a new patch to fix the harmless bogus
> pointer?

I don't think that's necessary, but maybe it would avoid people trying
to "fix" this code in the future. Jakub, WDYT?

-- 
Sabrina