linux-kernel - Re: [PATCH] nvmet-tcp: switch to using the crc32c library

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cfd791d0-f394-49bb-86a7-9f13614c4c24@grimberg.me>
Date: Thu, 27 Feb 2025 10:40:19 +0200
From: Sagi Grimberg <sagi@...mberg.me>
To: Hannes Reinecke <hare@...e.de>, Eric Biggers <ebiggers@...nel.org>
Cc: Christoph Hellwig <hch@....de>, Chaitanya Kulkarni <kch@...dia.com>,
 linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nvmet-tcp: switch to using the crc32c library



On 27/02/2025 9:26, Hannes Reinecke wrote:
> On 2/26/25 20:01, Eric Biggers wrote:
>> On Wed, Feb 26, 2025 at 10:37:55AM +0100, Hannes Reinecke wrote:
>>> ... and it also eliminates a sporadic crash which we've seen
>>> where 'snd_hash' wasn't initialized when sending PDUs.
>>> Thanks for doing this!
>>
>> I'm not sure how that could have happened, since the ahash was 
>> allocated when
>> 'if (queue->hdr_digest || queue->data_digest)' which seemed to match the
>> conditions for when it was used.  But yeah, it's certainly nice to 
>> not have the
>> pointless allocation to worry about.
>>
>>> (Note to self: check the nvme-tls code for crc32c usage ...)
>>
>> I have patches for nvme-tls almost ready too.  Just been taking my 
>> time since
>> I've been updating all other users of "crc32" and "crc32c" in the 
>> kernel too.
>> And I need to decide what to do about skb_copy_and_hash_datagram_iter().
>>
> If it were me I would _love_ to switch the nvme-tcp recv patch over to 
> recvmsg and kill the ->read_sock() implementation.
> ->read_sock uses a completely different codepath in tls_sw, and nvme is
> the only user of that. So there's a fair chance that we might miss any
> improvements or fixes.
>
> Plus we currently have no good way of handling TLS records from 
> ->read_sock(), which is something we might want to do in the future.
>
> So if we had an equivalent for skb_copy_and_hash_iter() for recvmsg()
> I could revisit my original patchset and work on getting ->read_sock()
> replaced.

Indeed, we want to calculate rolling crc32c as we copy the data vs. 
doing this afterwards...