linux-kernel - Re: [PATCH v8 3/5] rust: add bitmap API.

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]

Message-ID: <CAG48ez1NM7B8Vk7GOwhsitCipmfHi9eK6JNb3ve8aR4m8Cj0gA@mail.gmail.com>
Date: Mon, 19 May 2025 22:51:38 +0200
From: Jann Horn <jannh@...gle.com>
To: Burak Emir <bqe@...gle.com>
Cc: Yury Norov <yury.norov@...il.com>, Kees Cook <kees@...nel.org>, 
	Rasmus Villemoes <linux@...musvillemoes.dk>, Viresh Kumar <viresh.kumar@...aro.org>, 
	Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	"Gustavo A . R . Silva" <gustavoars@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v8 3/5] rust: add bitmap API.

On Mon, May 19, 2025 at 10:42 PM Burak Emir <bqe@...gle.com> wrote:
> On Mon, May 19, 2025 at 8:22 PM Yury Norov <yury.norov@...il.com> wrote:
> > On Mon, May 19, 2025 at 04:17:03PM +0000, Burak Emir wrote:
> > > +    /// Set bit with index `index`.
> > > +    ///
> > > +    /// ATTENTION: `set_bit` is non-atomic, which differs from the naming
> > > +    /// convention in C code. The corresponding C function is `__set_bit`.
> > > +    ///
> > > +    /// # Panics
> > > +    ///
> > > +    /// Panics if `index` is greater than or equal to `self.nbits`.
> > > +    #[inline]
> > > +    pub fn set_bit(&mut self, index: usize) {
> > > +        assert!(
> > > +            index < self.nbits,
> > > +            "Bit `index` must be < {}, was {}",
> > > +            self.nbits,
> > > +            index
> > > +        );
> >
> > Shouldn't this assertion be protected with hardening too? I already
> > said that: panicking on  out-of-boundary access with hardening
> > disabled is a wrong way to go.
>
> I considered it, but could not convince myself that __set_bit etc are
> actually always safe.
> For the methods that have the hardening assert, I was sure, but for
> this one, not.
>
> Are all bit ops guaranteed to handle out-of-bounds gracefully?
>
> > Can you turn your bitmap_hardening_assert() to just bitmap_assert(),
> > which panics only if hardening is enabled, and otherwise just prints
> > error with pr_err()?
>
> If there is no risk of undefined behavior, then I agree that checking
> bounds is hardening.
> If a missing bounds check loses safety, we then we should not skip it.

There are no bounds checks in these C APIs, and there can't be,
because the C side does not store a length. bitmap_zalloc() just gives
you a raw array of bits (represented in C as an array of unsigned
longs), it's a very lightweight wrapper around kmalloc_array().

And if you expand __set_bit(nr, addr), you'll see that it turns into:

bitop(___set_bit, nr, addr)

which turns into:

((__builtin_constant_p(nr) &&
  __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) &&
  (uintptr_t)(addr) != (uintptr_t)NULL &&
  __builtin_constant_p(*(const unsigned long *)(addr))) ?
const___set_bit(nr, addr) : ___set_bit(nr, addr))

which (assuming a non-constant index) is:

___set_bit(nr, addr)

which is a debug-instrumented wrapper around

arch___set_bit(nr, addr)

which just leads to a raw assembly instruction (example from x86):

static __always_inline void
arch___set_bit(unsigned long nr, volatile unsigned long *addr)
{
    asm volatile(__ASM_SIZE(bts) " %1,%0" : : ADDR, "Ir" (nr) : "memory");
}