linux-hardening - Re: Disabling -Warray-bounds for gcc-13 too

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: <CACO55tt7sGocRQPVv1vRh4X6Tjcu_C_=abi+C1Z6e2-deOFTCw@mail.gmail.com>
Date:   Fri, 28 Apr 2023 01:35:46 +0200
From:   Karol Herbst <kherbst@...hat.com>
To:     Lyude Paul <lyude@...hat.com>
Cc:     Kees Cook <kees@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Ben Skeggs <bskeggs@...hat.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        nouveau@...ts.freedesktop.org, linux-hardening@...r.kernel.org,
        gustavo@...eddedor.com, qing.zhao@...cle.com
Subject: Re: Disabling -Warray-bounds for gcc-13 too

On Fri, Apr 28, 2023 at 1:27 AM Lyude Paul <lyude@...hat.com> wrote:
>
> On Fri, 2023-04-28 at 00:50 +0200, Karol Herbst wrote:
> > On Fri, Apr 28, 2023 at 12:46 AM Lyude Paul <lyude@...hat.com> wrote:
> > >
> > > Hey Linus, Kees. Responses below
> > >
> > > On Sun, 2023-04-23 at 13:23 -0700, Kees Cook wrote:
> > > > On April 23, 2023 10:36:24 AM PDT, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> > > > > Kees,
> > > > >  I made the mistake of upgrading my M2 Macbook Air to Fedora-38, and
> > > > > in the process I got gcc-13 which is not WERROR-clean because we only
> > > > > limited the 'array-bounds' warning to gcc-11 and gcc-12. But gcc-13
> > > > > has all the same issues.
> > > > >
> > > > > And I want to be able to do my arm64 builds with WERROR on still...
> > > > >
> > > > > I guess it never made much sense to hope it was going to go away
> > > > > without having a confirmation, so I just changed it to be gcc-11+.
> > > >
> > > > Yeah, that's fine. GCC 13 released without having a fix for at least one (hopefully last) known array-bounds vs jump threading bug:
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109071
> > > >
> > > > > And one of them is from you.
> > > > >
> > > > > In particular, commit 4076ea2419cf ("drm/nouveau/disp: Fix
> > > > > nvif_outp_acquire_dp() argument size") cannot possibly be right, It
> > > > > changes
> > > > >
> > > > > nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[16],
> > > > >
> > > > > to
> > > > >
> > > > > nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
> > > > >
> > > > > and then does
> > > > >
> > > > >        memcpy(args.dp.dpcd, dpcd, sizeof(args.dp.dpcd));
> > > > >
> > > > > where that 'args.dp.dpcd' is a 16-byte array, and DP_RECEIVER_CAP_SIZE is 15.
> > > >
> > > > Yeah, it was an incomplete fix. I sent the other half here, but it fell through the cracks:
> > > > https://lore.kernel.org/lkml/20230204184307.never.825-kees@kernel.org/
> > >
> > > Thanks for bringing this to our attention, yeah this definitely just looks
> > > like it got missed somewhere down the line. It looks like Karol responded
> > > already so I assume the patch is in the pipeline now, but let me know if
> > > there's anything else you need.
> > >
> >
> > uhm, I didn't push anything, but I can push it through drm-misct asap,
> > just wanted to ask if somebody wants to pick a quicker route. But I
> > guess not?
>
> Ah whoops, I misunderstood! Yeah I would say we should just go ahead and push
> it since I don't see any indication here that anyone else has.
>

okay, will do so tomorrow then.

> >
> > > >
> > > >
> > > >
> > > > >
> > > >
> > > > > I think it's all entirely harmless from a code generation standpoint,
> > > > > because the 15-byte field will be padded out to 16 bytes in the
> > > > > structure that contains it, but it's most definitely buggy.
> > > >
> > > > Right; between this, that GCC 13 wasn't released yet, and I had no feedback from NV folks, I didn't chase down landing that fix.
> > > >
> > > > >
> > > > > So that warning does find real cases of wrong code. But when those
> > > > > real cases are hidden by hundreds of lines of unfixable false
> > > > > positives, we don't have much choice.
> > > >
> > > > Yup, totally agreed. The false positives I've looked at all seem to be similar to the outstanding jump threading bug, so I'm hoping once that gets fixed we'll finally have a good signal with that warning enabled. :)
> > > >
> > > > -Kees
> > > >
> > > >
> > >
> > > --
> > > Cheers,
> > >  Lyude Paul (she/her)
> > >  Software Engineer at Red Hat
> > >
> >
>
> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
>