linux-kernel - Re: [RFC PATCH 1/3] modpost: fix TO_NATIVE() with expressions and consts

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: <Yv5ITarFK9Z3bkhZ@kroah.com>
Date:   Thu, 18 Aug 2022 16:10:21 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
Cc:     linux-kernel@...r.kernel.org,
        Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Kosina <jikos@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Petr Mladek <pmladek@...e.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        linux-kbuild@...r.kernel.org, live-patching@...r.kernel.org,
        lkp@...el.com, stable@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] modpost: fix TO_NATIVE() with expressions and
 consts

On Thu, Aug 18, 2022 at 04:01:53PM +0200, Alexander Lobakin wrote:
> From: Greg KH <gregkh@...uxfoundation.org>
> Date: Thu, 18 Aug 2022 14:26:14 +0200
> 
> > On Thu, Aug 18, 2022 at 01:53:04PM +0200, Alexander Lobakin wrote:
> > > Macro TO_NATIVE() directly takes a reference to its argument @x
> > > without making an intermediate variable. This makes compilers
> > > emit build warnings and errors if @x is an expression or a deref
> > > of a const pointer (when target Endianness != host Endianness):
> > > 
> > > >> scripts/mod/modpost.h:87:18: error: lvalue required as unary '&' operand
> > >       87 |         __endian(&(x), &(__x), sizeof(__x));                    \
> > >          |                  ^
> > >    scripts/mod/sympath.c:19:25: note: in expansion of macro 'TO_NATIVE'
> > >       19 | #define t(x)            TO_NATIVE(x)
> > >          |                         ^~~~~~~~~
> > >    scripts/mod/sympath.c:100:31: note: in expansion of macro 't'
> > >      100 |                 eh->e_shoff = t(h(eh->e_shoff) + off);
> > > 
> > > >> scripts/mod/modpost.h:87:24: warning: passing argument 2 of '__endian'
> > > discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> > >       87 |         __endian(&(x), &(__x), sizeof(__x));                    \
> > >          |                        ^~~~~~
> > >    scripts/mod/sympath.c:18:25: note: in expansion of macro 'TO_NATIVE'
> > >       18 | #define h(x)            TO_NATIVE(x)
> > >          |                         ^~~~~~~~~
> > >    scripts/mod/sympath.c:178:48: note: in expansion of macro 'h'
> > >      178 |              iter < end; iter = (void *)iter + h(eh->e_shentsize)) {
> > 
> > How come this hasn't shown up in cross-builds today?
> 
> It doesn't happen with the current code.

Great, so there is no bug that you are trying to fix :)

> > > Create a temporary variable, assign @x to it and don't use @x after
> > > that. This makes it possible to pass expressions as an argument.
> > > Also, do a cast-away for the second argument when calling __endian()
> > > to avoid 'discarded qualifiers' warning, as typeof() preserves
> > > qualifiers and makes compilers think that we're passing pointer
> > > to a const.
> > > 
> > > Reported-by: kernel test robot <lkp@...el.com>
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Cc: stable@...r.kernel.org # 4.9+
> > 
> > Where are these build warnings showing up at that we don't see them
> > today, yet this is needed to go back to all stable trees?
> 
> I thought all fixes should go to the applicable stable trees, am I
> wrong? If so, I'll drop the tag in the next spin.

But this isn't fixing a bug in the code today that anyone can hit, so
why would you mark it as such?

> I remember we had such discussion already regarding fixing stuff in
> modpost, which can happen only with never mainlained GCC LTO or with
> the in-dev code. At the end that fix made it into the stables IIRC.

I don't remember taking fixes for out-of-tree LTO stuff, but I shouldn't
have :)

thanks,

greg k-h