From: Kees Cook <keescook@chromium.org>
To: Nathan Chancellor <natechancellor@gmail.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
LKML <linux-kernel@vger.kernel.org>,
clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH] ubsan: Implement __ubsan_handle_alignment_assumption
Date: Tue, 12 Jan 2021 15:56:26 -0800 [thread overview]
Message-ID: <202101121555.C929315@keescook> (raw)
In-Reply-To: <20210112220634.GA1377517@ubuntu-m3-large-x86>
On Tue, Jan 12, 2021 at 03:06:34PM -0700, Nathan Chancellor wrote:
> On Tue, Jan 12, 2021 at 01:53:30PM -0800, Nick Desaulniers wrote:
> > On Tue, Jan 12, 2021 at 1:37 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > > if real_ptr is an unsigned long, do we want to use `__ffs(real_ptr) +
> > > > 1` here rather than ffs which takes an int? It seems the kernel is
> > > > missing a definition of ffsl. :(
> > >
> > > Why the + 1? I think if we use __ffs (which it seems like we should), I
> > > think that needs to become
> >
> > This came up recently in an internal code review; ffs and __ffs differ
> > in output by one. See also the definition of ffs for alpha in
> > arch/alpha/include/asm/bitops.h.
>
> Interesting, thanks for bringing it up! Looks like ffs returns 1-32 and
> __ffs returns 0-31. I think that we want __ffs here because we are
> shifting (1UL << 32 overflows on 32-bit architectures) and the code in
> LLVM appears to agree. LeastSignificantSetBitIndex evaluates to
> __builtin_ctzl, which is the asm-generic implementation of __ffs.
Sounds good. With __ffs, consider your v2:
Acked-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
next prev parent reply other threads:[~2021-01-13 0:52 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-12 20:55 [PATCH] ubsan: Implement __ubsan_handle_alignment_assumption Nathan Chancellor
2021-01-12 21:15 ` Nick Desaulniers
2021-01-12 21:37 ` Nathan Chancellor
2021-01-12 21:53 ` Nick Desaulniers
2021-01-12 22:06 ` Nathan Chancellor
2021-01-12 23:56 ` Kees Cook [this message]
2021-01-13 0:12 ` [PATCH v2] " Nathan Chancellor
2021-01-27 22:44 ` [PATCH v3] " Nathan Chancellor
2021-01-27 22:54 ` Nick Desaulniers
2021-01-13 0:18 ` [PATCH] " kernel test robot
2021-01-13 0:18 ` kernel test robot
2021-01-13 0:39 ` kernel test robot
2021-01-13 0:39 ` kernel test robot
2021-01-13 1:31 ` Nathan Chancellor
2021-01-13 1:39 ` Nick Desaulniers
2021-01-13 1:39 ` Nick Desaulniers
2021-01-13 1:39 ` Nick Desaulniers
2021-01-27 22:26 ` Nick Desaulniers
2021-01-27 22:26 ` Nick Desaulniers
2021-01-27 22:26 ` Nick Desaulniers
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=202101121555.C929315@keescook \
--to=keescook@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.