netdev - Re: UBSAN: annotation to skip sanitization in variable that will wrap

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

Message-ID: <CAFhGd8oowe7TwS88SU1ETJ1qvBP++MOL1iz3GrqNs+CDUhKbzg@mail.gmail.com>
Date: Wed, 14 Aug 2024 14:05:49 -0700
From: Justin Stitt <justinstitt@...gle.com>
To: Breno Leitao <leitao@...ian.org>
Cc: kees@...nel.org, elver@...gle.com, andreyknvl@...il.com, 
	ryabinin.a.a@...il.com, kasan-dev@...glegroups.com, 
	linux-hardening@...r.kernel.org, axboe@...nel.dk, asml.silence@...il.com, 
	netdev@...r.kernel.org
Subject: Re: UBSAN: annotation to skip sanitization in variable that will wrap

Hi,

On Wed, Aug 14, 2024 at 10:10 AM Breno Leitao <leitao@...ian.org> wrote:
>
> Hello,
>
> I am seeing some signed-integer-overflow in percpu reference counters.

it is brave of you to enable this sanitizer :>)

>
>         UBSAN: signed-integer-overflow in ./arch/arm64/include/asm/atomic_lse.h:204:1
>         -9223372036854775808 - 1 cannot be represented in type 's64' (aka 'long long')
>         Call trace:
>
>          handle_overflow
>          __ubsan_handle_sub_overflow
>          percpu_ref_put_many
>          css_put
>          cgroup_sk_free
>          __sk_destruct
>          __sk_free
>          sk_free
>          unix_release_sock
>          unix_release
>          sock_close
>
> This overflow is probably happening in percpu_ref->percpu_ref_data->count.
>
> Looking at the code documentation, it seems that overflows are fine in
> per-cpu values. The lib/percpu-refcount.c code comment says:
>
>  * Note that the counter on a particular cpu can (and will) wrap - this
>  * is fine, when we go to shutdown the percpu counters will all sum to
>  * the correct value
>
> Is there a way to annotate the code to tell UBSAN that this overflow is
> expected and it shouldn't be reported?

Great question.

1) There exists some new-ish macros in overflow.h that perform
wrapping arithmetic without triggering sanitizer splats -- check out
the wrapping_* suite of macros.

2) I have a Clang attribute in the works [1] that would enable you to
annotate expressions or types that are expected to wrap and will
therefore silence arithmetic overflow/truncation sanitizers. If you
think this could help make the kernel better then I'd appreciate a +1
on that PR so it can get some more review from compiler people! Kees
and I have some other Clang features in the works that will allow for
better mitigation strategies for intended overflow in the kernel.

3) Kees can probably chime in with some other methods of getting the
sanitizer to shush -- we've been doing some work together in this
space. Also check out [2]

>
> Thanks
> --breno
>
>

[1]: https://github.com/llvm/llvm-project/pull/86618
[2]: https://lwn.net/Articles/979747/ (Arithmetic overflow mitigation
in the kernel; Jul 1 2024)