Re: [PATCH net-next v10 08/16] tls: Inline do_tcp_sendpages()

From: Tariq Toukan <ttoukan.linux@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: David Howells <dhowells@redhat.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	David Ahern <dsahern@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Jens Axboe <axboe@kernel.dk>, Jeff Layton <jlayton@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	Chuck Lever III <chuck.lever@oracle.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Boris Pismenny <borisp@nvidia.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Gal Pressman <gal@nvidia.com>,
	ranro@nvidia.com, samiram@nvidia.com, drort@nvidia.com,
	Tariq Toukan <tariqt@nvidia.com>
Subject: Re: [PATCH net-next v10 08/16] tls: Inline do_tcp_sendpages()
Date: Tue, 8 Aug 2023 10:29:18 +0300	[thread overview]
Message-ID: <ca5ac2f8-d3b1-0d07-d361-6ce1d4f12392@gmail.com> (raw)
In-Reply-To: <20230803201212.1d5dd0f9@kernel.org>

On 04/08/2023 6:12, Jakub Kicinski wrote:
> On Thu, 3 Aug 2023 14:47:35 +0300 Tariq Toukan wrote:
>> When applying this patch, repro disappears! :)
>> Apparently it is related to the warning.
>> Please go on and submit it.
> 
> I have no idea how. I found a different bug, staring at this code
> for another hour. But I still don't get how we can avoid UaF on
> a page by having the TCP take a ref on it rather than copy it.
> 
> If anything we should have 2 refs on any page in the sg, one because
> it's on the sg, and another held by the re-tx handling.
> 
> So I'm afraid we're papering over something here :( We need to keep
> digging.

Hi Jakub,
I'm glad to see that you already nailed the other bug and merged the fix.

I can update that we ran comprehensive TLS testing on a branch that 
contains your proposed fix (net: tls: set MSG_SPLICE_PAGES 
consistently), and doesn't contain the other fix (net: tls: avoid 
discarding data on record close).

Except for one "known" issue (we'll discuss it in a second), the runs 
look clean.
No more traces or encrypt/decrypt error counters. Your proposed fix 
seems to work and causes no degradation.
How do you suggest proceeding here?

One mysterious remaining issue, which I already reported some time ago 
but couldn't effectively debug due to other TLS bugs, is the increase of 
TlsDecryptError / TlsEncryptError counters when running kTLS offloaded 
traffic during bond creation on some other interface.
Weird...

We should start giving it the needed attention now that the other issues 
seem to be resolved.

Regards,
Tariq