[Nouveau] Disabling -Warray-bounds for gcc-13 too

* [Nouveau] Disabling -Warray-bounds for gcc-13 too
@ 2023-04-23 17:36 Linus Torvalds
  2023-04-23 20:23 ` Kees Cook
  0 siblings, 1 reply; 8+ messages in thread
From: Linus Torvalds @ 2023-04-23 17:36 UTC (permalink / raw)
  To: Kees Cook; +Cc: nouveau, dri-devel, Ben Skeggs, Daniel Vetter

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+.

A lot of the warnings seem just crazy, with gcc just not getting the
bounds right, and then being upset about us going backwards with
'container_of()' etc. Ok, so the kernel is special. We do odd things.
I get it, gcc ends up being confused.

But before I disabled it, I did take a look at a couple of warnings
that didn't look like the sea of crazy.

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.

So yeah, it's copying 16 bytes from an argument that claims to be 15
bytes in size.

I think that commit was wrong, and the problem is that the 'dpcd'
array is something 15 and sometimes 16. For example, we have

  struct nouveau_encoder {
        ...
        union {
            struct {
            ...
                u8 dpcd[DP_RECEIVER_CAP_SIZE];
            } dp;
        };

so there it's indeed 15 bytes, but then we have

union nvif_outp_acquire_args {
        struct nvif_outp_acquire_v0 {
            ...
            union {
                ...
                struct {
                    ...
                    __u8 dpcd[16];
                } dp;

where it's 16.

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.

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.

But could the Nouveau driver *please* pick a size for the dhcp[] array
and stick with it?

The other driver where the warnings didn't look entirely crazy was the
ath/carl9170 wireless driver, but I didn't look closer at that one.

                 Linus

^ permalink raw reply	[flat|nested] 8+ messages in thread