linux-hardening - Re: [PATCH v1] x86: suppress warning generated by W=1

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: <CR2LBKD251YR.2LJHOHOX6WPO0@vincent-arch>
Date:   Fri, 10 Mar 2023 10:19:53 +0100
From:   "Vincenzo Palazzo" <vincenzopalazzodev@...il.com>
To:     "Greg KH" <gregkh@...uxfoundation.org>
Cc:     <x86@...nel.org>, <luto@...nel.org>,
        <linux-hardening@...r.kernel.org>
Subject: Re: [PATCH v1] x86: suppress warning generated by W=1

On Fri Mar 10, 2023 at 8:33 AM CET, Greg KH wrote:
> On Thu, Mar 09, 2023 at 06:48:54PM +0100, Vincenzo Palazzo wrote:
> > suppress unused warnings and fix the error that there is
> > with the W=1 enabled.
>
> Why are you building with that option enabled?  It's not a normal one at
> all.

I was using this option because I would like to see if my c code has
warnings, but currently it is not possible compile the kernel with 
W=1.

Maybe I'm missing a little bit of history here, this is not the correct
option to compile the kernel with -Wall?

Or it is the wrong way to compile the kernel with -Wall, so at this
point another good patch if we do not want to see the warnings, is
rework the W=1 to allow a subset of warnings, like the one in this
patch?

>
> > 
> > Warning generated
> > 
> > arch/x86/entry/common.c:238:24: error: no previous prototype for ‘do_SYSENTER_32’ [-Werror=missing-prototypes]
> >   238 | __visible noinstr long do_SYSENTER_32(struct pt_regs *regs)
> > 
> > Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@...il.com>
> > ---
> >  arch/x86/entry/common.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index 6c2826417b33..8f17b2c3e9de 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -234,6 +234,9 @@ __visible noinstr long do_fast_syscall_32(struct pt_regs *regs)
> >  #endif
> >  }
> >  
> > +/* prototype is a placeholder to suppress the missing prototype worning */
> > +long do_SYSENTER_32(struct pt_regs *regs);
>
> This feels wrong, sorry, and you have a spelling error in your comment.

Yeah it is, in fact this do not make sense because we are in a c file.
However, looks like it is the only option to suppress the warnings in
this case?

Do you know some magic __attribute__ that can suppress it for a not
static function like the one in this PATCH?

>
> > +
> >  /* Returns 0 to return using IRET or 1 to return using SYSEXIT/SYSRETL. */
> >  __visible noinstr long do_SYSENTER_32(struct pt_regs *regs)
>
> It's wrong because look, you define it right here so why do you need a
> prototype?

Here the problem is the macros __visible that force the function to be 
not static because need to be visible outside, and in C as far I know a 
procedure without a prototype need to be static, for this reason I made the 
previous trick to suppress the warning.

I thought also to modify the macros in some way but I did not have a
good idea.

P.S: Thanks to catch the typo, I will provide a v2 when we converge in a
common solution.

Cheers!

Vincent.