linux-kernel - Re: drm/nouveau/bios/ramcfg, setting of RON pull value

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

Date:   Sat, 16 Feb 2019 11:02:49 -0500
From:   Ilia Mirkin <imirkin@...m.mit.edu>
To:     Colin Ian King <colin.king@...onical.com>
Cc:     Roy Spliet <rspliet@...ipso.eu>, Ben Skeggs <bskeggs@...hat.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        nouveau <nouveau@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: drm/nouveau/bios/ramcfg, setting of RON pull value

On Sat, Feb 16, 2019 at 10:02 AM Colin Ian King
<colin.king@...onical.com> wrote:
>
> Hi,
>
> Static Analysis with CoverityScan as detected an issue with the setting
> of the RON pull value in function nvkm_gddr3_calc in
> drm/nouveau/bios/ramcfg.c
>
> This was introduced by commit: c25bf7b6155cb ("drm/nouveau/bios/ramcfg:
> Separate out RON pull value")
>
> CoverityScan reports the issue as follows:
>
>  84        case 0x20:
>  85                CWL = (ram->next->bios.timing[1] & 0x00000f80) >> 7;
>  86                CL  = (ram->next->bios.timing[1] & 0x0000001f) >> 0;
>  87                WR  = (ram->next->bios.timing[2] & 0x007f0000) >> 16;
>  88                /* XXX: Get these values from the VBIOS instead */
>  89                DLL = !(ram->mr[1] & 0x1);
>
>    CID 1324005 (#1 of 1): Operands don't affect result
> (CONSTANT_EXPRESSION_RESULT)
>
> result_independent_of_operands: !(ram->mr[1] & 768) >> 8 is 0 regardless
> of the values of its operands. This occurs as the operand of assignment.
>
>  90                RON = !(ram->mr[1] & 0x300) >> 8;
>  91                break;
>
> Looking at this, I believe perhaps the correct setting could be:
>
> RON = !((ram->mr[1] & 0x300) >> 8);
>
> ..however I don't have the datasheet available for the H/W so I'm not
> sure if this a correct fix.

Actually looking at the code a bit, I suspect it should just be

RON = (ram->mr[1] & 0x300) >> 8;

since later on, when we recompose the MR (memory register) value, we do:

        ram->mr[1] |= (RON & 0x03) << 8;

(And the whole point here is that we don't know how to get the proper
RON value for that timing table version, so we just copy whatever used
to be there in that case.)

  -ilia