linux-kernel - Re: [PATCH mips-fixes] MIPS: fix fortify panic when copying asm exception handlers

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: <20220304234818.153517-1-alobakin@pm.me>
Date:   Fri, 04 Mar 2022 23:55:30 +0000
From:   Alexander Lobakin <alobakin@...me>
To:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>
Cc:     Alexander Lobakin <alobakin@...me>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Mike Rapoport <rppt@...nel.org>,
        Davidlohr Bueso <dbueso@...e.de>,
        Florian Fainelli <f.fainelli@...il.com>,
        Liam Howlett <liam.howlett@...cle.com>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Atsushi Nemoto <anemo@....ocn.ne.jp>,
        linux-mips@...r.kernel.org, stable@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH mips-fixes] MIPS: fix fortify panic when copying asm exception handlers

From: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
Date: Tue, 1 Mar 2022 17:34:11 +0100

> On Wed, Feb 23, 2022 at 01:30:23AM +0000, Alexander Lobakin wrote:
> > With KCFLAGS="-O3", I was able to trigger a fortify-source
> > memcpy() overflow panic on set_vi_srs_handler().
> > Although O3 level is not supported in the mainline, under some
> > conditions that may've happened with any optimization settings,
> > it's just a matter of inlining luck. The panic itself is correct,
> > more precisely, 50/50 false-positive and not at the same time.
> > >From the one side, no real overflow happens. Exception handler
> > defined in asm just gets copied to some reserved places in the
> > memory.
> > But the reason behind is that C code refers to that exception
> > handler declares it as `char`, i.e. something of 1 byte length.
> > It's obvious that the asm function itself is way more than 1 byte,
> > so fortify logics thought we are going to past the symbol declared.
> > The standard way to refer to asm symbols from C code which is not
> > supposed to be called from C is to declare them as
> > `extern const u8[]`. This is fully correct from any point of view,
> > as any code itself is just a bunch of bytes (including 0 as it is
> > for syms like _stext/_etext/etc.), and the exact size is not known
> > at the moment of compilation.
> > Adjust the type of the except_vec_vi_*() and related variables.
> > Make set_handler() take `const` as a second argument to avoid
> > cast-away warnings and give a little more room for optimization.
> >
> > Fixes: e01402b115cc ("More AP / SP bits for the 34K, the Malta bits and things. Still wants")
> > Fixes: c65a5480ff29 ("[MIPS] Fix potential latency problem due to non-atomic cpu_wait.")
> > Cc: stable@...r.kernel.org # 3.10+
>
> I like your patch, but I have a problem with these tags. If I understand
> your description correctly there is no bug, but because of the way the
> code is written fortify-source gets confused. So if it doesn't fix
> anything, there shouldn't be Fixes tags, IMHO. If you agree, I'll
> apply this patch to mips-next and remove the tags.

Oh, sorry for the late reply.
Sure, feel free to apply to mips-next. Yeah, there is no real bug,
just not-really-100%-clean-code which works anyways.

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

Thanks,
Al