From: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Daniel Xu <dxu@dxuuu.xyz>, Matt Turner <mattst88@gmail.com>,
Richard Henderson <rth@twiddle.net>, bpf <bpf@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Song Liu <songliubraving@fb.com>,
andrii.nakryiko@gmail.com, kernel-team@fb.com
Subject: Re: [PATCH bpf v6 1/2] lib/strncpy_from_user.c: Don't overcopy bytes after NUL terminator
Date: Tue, 17 Nov 2020 18:22:15 +0000 [thread overview]
Message-ID: <20201117182215.GA15956@mail.rc.ru> (raw)
In-Reply-To: <CAHk-=wiEgTXYgLXg8YxRHnH+eZno800pEp8caskKgDCgq55s+g@mail.gmail.com>
On Mon, Nov 16, 2020 at 02:44:56PM -0800, Linus Torvalds wrote:
> On Mon, Nov 16, 2020 at 2:15 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So I've verified that at least on x86-64, this doesn't really make
> > code generation any worse, and I'm ok with the patch from that
> > standpoint.
>
> .. looking closer, it will generate extra code on big-endian
> architectures and on alpha, because of the added "zero_bytemask()".
> But on the usual LE machines, zero_bytemask() will already be the same
> as "mask", so all it adds is that "and" operation with values it
> already had access to.
>
> I don't think anybody cares about alpha and BE - traditional BE
> architectures have moved to LE anyway. And looking at the alpha
> word-at-a-time code, I don't even understand how it works at all.
>
> Adding matt/rth/ivan to the cc, just so that maybe one of them can
> educate me on how that odd alpha zero_bytemask() could possibly work.
> The "2ul << .." part confuses me, I think it should be "1ul << ...".
>
> I get the feeling that the alpha "2ul" constant might have come from
> the tile version, but in the tile version, the " __builtin_ctzl()"
> counts the leading zeroes to the top bit of any bytes in 'mask'. But
> the alpha version actually uses "find_zero(mask) * 8", so rather than
> have values of 7/15/23/... (for zero byte in byte 0/1/2/..
> respectively), it has values 0/8/16/....
>
> But it's entirely possible that I'm completely confused, and alpha
> does it right, and I'm just not understanding the code.
No, you are right, it should be "1ul". Indeed, seems like it came from
the tile version which looks incorrect either, BTW. The tile-gx ISA
(https://studylib.net/doc/18755547/tile-gx-instruction-set-architecture)
says that clz/ctz instructions count up to the first "1", not to the
last "0", so the shift values in tile's zero_bytemask() are 0/8/16/...
as well.
> It's also possible that the "2ul" vs "1ul" case doesn't matter.
> because the extra bit is always going to mask the byte that is
> actually zero, so being one bit off in the result is a non-event. I
> think that is what may actually be going on.
Yes, looks like that.
Ivan.
next prev parent reply other threads:[~2020-11-17 18:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 21:17 [PATCH bpf v6 0/2] Fix bpf_probe_read_user_str() overcopying Daniel Xu
2020-11-16 21:17 ` [PATCH bpf v6 1/2] lib/strncpy_from_user.c: Don't overcopy bytes after NUL terminator Daniel Xu
2020-11-16 22:15 ` Linus Torvalds
2020-11-16 22:44 ` Linus Torvalds
2020-11-17 18:22 ` Ivan Kokshaysky [this message]
2020-11-17 4:53 ` Daniel Xu
2020-11-16 21:17 ` [PATCH bpf v6 2/2] selftest/bpf: Test bpf_probe_read_user_str() strips trailing bytes after NUL Daniel Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201117182215.GA15956@mail.rc.ru \
--to=ink@jurassic.park.msu.ru \
--cc=andrii.nakryiko@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dxu@dxuuu.xyz \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mattst88@gmail.com \
--cc=rth@twiddle.net \
--cc=songliubraving@fb.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox