linux-kernel - Re: [PATCH] lib/string: shrink lib/string.i via IWYU

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: <ZW-b4l1llqPXW-el@smile.fi.intel.com>
Date:   Tue, 5 Dec 2023 23:53:38 +0200
From:   Andy Shevchenko <andy@...nel.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, tanzirh@...gle.com,
        Kees Cook <keescook@...omium.org>,
        linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
        Nick DeSaulniers <nnn@...gle.com>, llvm@...ts.linux.dev
Subject: Re: [PATCH] lib/string: shrink lib/string.i via IWYU

On Tue, Dec 05, 2023 at 01:39:47PM -0800, Nick Desaulniers wrote:
> On Tue, Dec 5, 2023 at 1:24 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
> > On Tue, 5 Dec 2023 13:14:16 -0800 Nick Desaulniers <ndesaulniers@...gle.com> wrote:
> > > > The preferred way to import bit-fiddling stuff is to include
> > > > <linux/bits.h>.  Under the hood this may include asm/bitsperlong.h.  Or
> > > > it may not, depending on Kconfig settings (particularly architecture).
> > >
> > > Just triple checking my understanding; it looks like
> > > include/linux/bits.h unconditionally includes asm/bitsperlong.h (which
> > > is implemented per arch) most of which seem to include
> > > asm-generic/bitsperlong.h.
> > >
> > > include/linux/bits.h also defines a few macros (BIT_MASK, BIT_WORD,
> > > BITS_PER_BYTE, GENMASK, etc).  If lib/string.c is not using any of
> > > those, why can't we go straight to #including asm/bitsperlong.h?  That
> > > should resolve to the arch specific impl which may include
> > > asm-generic/bitsperlong.h?
> >
> > It's just a general rule.  If the higher-level include is present, use
> > that.  Because of the above, plus I guess things might change in the
> > future.
> 
> Hmm...how does one know that linux/bits.h is the higher-level include
> of asm/bitsperlong.h?
> 
> Do we mention these conventions anywhere under Documentation?

Unfortunately it comes with an experience. The problem here is the absence of
Documentation for the headers that guarantee inclusion of other headers. In
general linux/* are preferred over asm/* but sometimes things are complicated
(when some asm ones include linux ones via architectural code and circling
around).

> > We've been getting better about irregular asm/include files.
> >
> > But bits.h is a poor example.  A better case to study is spinlock.h.
> > If this tool recommended including asm/spinlock.h then that won't work
> > on any architecture which doesn't implement SMP (there is no
> > arch/nios2/include/asm/spinlock.h).
> 
> The tooling Tanzir is working on does wrap IWYU, and does support such
> mapping (of 'low level' to 'high level' headers; more so, if it
> recommends X you can override to suggest Y instead).
> 
> arch/nios/ also doesn't provide a bug.h, which this patch is
> suggesting we include directly.  I guess the same goes for
> asm/rwonce.h.

Have you checked Ingo Molnar's gigantic series (2k+ patches) for the header
hell clean up? Perhaps we need to apply that first.

-- 
With Best Regards,
Andy Shevchenko