netdev - Re: [PATCH net 1/1] net: sched: cls_u32: Fix match key mis-addressing

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHk-=wi43Y7Osoa2NYr2FFxsajLjJUJSgAUdJmVPJfV8ggFYRg@mail.gmail.com>
Date: Tue, 25 Jul 2023 10:44:29 -0700
From: Linus Torvalds <torvalds@...uxfoundation.org>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com, 
	pabeni@...hat.com, jiri@...nulli.us, xiyou.wangcong@...il.com, 
	netdev@...r.kernel.org, mgcho.minic@...il.com, security@...nel.org
Subject: Re: [PATCH net 1/1] net: sched: cls_u32: Fix match key mis-addressing

On Tue, 25 Jul 2023 at 10:04, Jamal Hadi Salim <jhs@...atatu.com> wrote:
>
> > I would also like to see some explanation of this pattern
> >
> >                 handle = htid | TC_U32_NODE(handle);
> >
> > and why that "binary or" makes sense. Are the node bits in 'htid'
> > guaranteed to be zero?
>
> Per existing user space tools, yes - they are guaranteed to be zero
> (per my gitchelogy of both kernel +  iproute2 since inception this has
> been the behavior);

Ok, if the htid bits are zero, it's all good.  It's fine to use a
binary 'or' as a way to 'insert bits in the word', but only if the old
bits were zero, and that wasn't obvious to me.

The *normal* pattern would be to explicitly mask off the bits you want
to use, so that you don't get some random mixing of bits of the
fields.

Of course, that's a bit inconvenient here, since you don't have the
obvious accessors.

And while using bitfields would make the source code look fine,
bitfields are *horrible* for any ABI, since they have very weakly
defined semantics (ie litte-endian vs big-endian, and the *bit* order
is not at all guaranteed to match the *byte* order).

> > Because if 'htid' can have node bits set, what's the logical reason
> > for or'ing things together?
>
> Hrm. I am not sure if this is what you are getting to: but you caught
> a different bug there.

So what I'm getting at was just that *if* you can have mixing of bits
of that NODE part of the variable, I think the end result doesn't end
up being very sensible.

It's not that 'binary or' isn't a valid operation. But it normally
isn't all that sane to randomly just mix bits in these kinds of
things. What would it mean to mix node bits from an old value with the
user-supplied one?

So that pattern just looked odd to me.

                     Linus