On Fri, Apr 28, 2023 at 12:46?AM Lyude Paul <lyude at redhat.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 at
linux-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 at
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?
> >
> >
> >
> > >
> >
> > > 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
>