netdev - Re: [PATCH net-next 4/4] net: skbuff: fix l4

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]

Date: Tue, 16 May 2023 09:36:42 +0200
From: Antoine Tenart <atenart@...nel.org>
To: Dumitru Ceara <dceara@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Ilya Maximets <i.maximets@....org>
Cc: i.maximets@....org, davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/4] net: skbuff: fix l4_hash comment

Quoting Ilya Maximets (2023-05-15 20:23:28)
> On 5/15/23 10:12, Antoine Tenart wrote:
> > Quoting Dumitru Ceara (2023-05-11 22:50:32)
> >> On 5/11/23 19:54, Ilya Maximets wrote:
> >>>>> Note that skb->hash has never been considered as canonical, for obvious reasons.
> >>>
> >>> I guess, the other point here is that it's not an L4 hash either.
> >>>
> >>> It's a random number.  So, the documentation will still not be
> >>> correct even after the change proposed in this patch.
> > 
> > The proposed changed is "indicate hash is from layer 4 and provides a
> > uniform distribution over flows", which does not describe *how* the hash
> > is computed but *where* it comes from. This matches "random number set
> > by TCP" and changes in how hashes are computed won't affect the comment,
> > so we'll not end up in the same situation.
> 
> I respectfully disagree,  "is from layer 4" and "random number" do not
> match for me.  So, "where it comes from" argument is not applicable.
> Random numbers come from random number generator, and not "from layer 4".
> 
> Unless by "from layer 4" you mean "from the code that handles layer 4
> packet processing".  But that seems very confusing to me.  And it is
> definitely not the first thing that comes to mind while reading the
> documentation.

Yes that is what I meant, but if that is still confusing then this is
not improving things so let's try something better. I intentionally did
not mention how the hash is computed because it's easy to forget to
update the documentation when the exact logic is changed. What's
important here IMHO is to mention what the hash provides.

What about "indicates hash was set by layer 4 stack and provides a
uniform distribution over flows"? Or/and we should we also add a
disclaimer like "no guarantee on how the hash was computed"?

> Also making it look like subsystems that use it in a previously
> documented meaning are at fault.  It's just not fair.

I never said that nor it is what I think, I'm sorry if my message was
misleading.

Antoine