linux-kernel - Re: [git pull] drm next pull for 5.10-rc1

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: <CAPM=9tzwRb13Umsgq+aqo2ZDqjEc9578tS7_ydxkYPYArmSong@mail.gmail.com>
Date:   Fri, 16 Oct 2020 05:13:38 +1000
From:   Dave Airlie <airlied@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Eryk Brol <eryk.brol@....com>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [git pull] drm next pull for 5.10-rc1

On Fri, 16 Oct 2020 at 04:42, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Thu, Oct 15, 2020 at 10:51 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > Thanks, looks good to me [..]
>
> Uhhuh. I already pushed things out, but my clang build (which I don't
> do between each merge) shows a problem:
>
>   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:650:8:
>   warning: logical not is only applied to the left hand side of this
> comparison [-Wlogical-not-parentheses]
>                 && !params[i].clock_force_enable == DSC_CLK_FORCE_DEFAULT) {
>                    ^                             ~~
>
> and I think clang is entirely right to complain about that code.
>
> Yes, the code may be correct, but even if it's correct, that's a
> really odd way to write things.
>
> Anyway, what it does is:
>
>    !params[i].clock_force_enable
>
> turns 0 into 1, and anything that isn't 0 into 0.
>
> And DSC_CLK_FORCE_DEFAULT has a value of 0, so what that line actually means is
>
>   (params[i].clock_force_enable == 0) == 0
>
> which obviously is
>
>   params[i].clock_force_enable != 0
>
> which in this case is the same as
>
>   params[i].clock_force_enable != DSC_CLK_FORCE_DEFAULT
>
> which may be what the code actually meant to do.
>
> So I suspect it does what it meant to do, but only because
> DSC_CLK_FORCE_DEFAULT also happens to be 0, which also acts as a
> boolean 'false'.
>
> But it's also possible that the '!' is a left-over, and the code
> actually _meant_ to do the exact reverse of that. I have no idea.
>
> This odd code was introduced by commit 0749ddeb7d6c ("drm/amd/display:
> Add DSC force disable to dsc_clock_en debugfs entry"), and can we
> please agree to not write this kind of obfuscated C code?

I've asked Alex to direct send you any fix for you to apply once he's
had a chance to validate it,

Dave.