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-next>] [day] [month] [year] [list]

Date:   Sun, 23 Apr 2023 13:23:48 -0700
From:   Kees Cook <kees@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>
CC:     Ben Skeggs <bskeggs@...hat.com>, Karol Herbst <kherbst@...hat.com>,
        Lyude Paul <lyude@...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 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/



>

>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


-- 
Kees Cook