linux-kernel - Re: [PATCH v3] drm/dp

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: <f64197e6-74bd-6577-2aa7-9c69cfdb9080@st.com>
Date:   Fri, 31 Jan 2020 08:08:34 +0000
From:   Benjamin GAIGNARD <benjamin.gaignard@...com>
To:     Dave Airlie <airlied@...il.com>, Lyude Paul <lyude@...hat.com>
CC:     Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        David Airlie <airlied@...ux.ie>,
        ML dri-devel <dri-devel@...ts.freedesktop.org>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        Sean Paul <sean@...rly.run>
Subject: Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings


On 1/31/20 12:22 AM, Dave Airlie wrote:
>>>> hi-actually yes, we should probably be using this instead of just dropping
>>>> this. Also, I didn't write this code originally I just refactored a bunch
>>>> of
>>>> it - Dave Airlied is the original author, but the original version of this
>>>> code was written ages ago. tbh, I think it's a safe bet to say that they
>>>> probably did mean to use this but forgot to and no one noticed until now.
>>> Hi,
>>>
>>> Any clue about how to use crc value ? Does it have to be checked
>>> against something else ?
>>> If crc are not matching what should we do of the data copied just before ?
>> We should be able to just take the CRC value from the sideband message and
>> then generate our own CRC value using the sideband message contents, and check
>> if the two are equal. If they aren't, something went wrong and we didn't
>> receive the message properly.
>>
>> Now as to what we should do when we have CRC mismatches? That's a bit more
>> difficult. If you have access to the DP MST spec, I suppose a place to start
>> figuring that out would be checking if there's a way for us to request that a
>> branch device resend whatever message it sent previously. If there isn't, I
>> guess we should just print an error in dmesg (possibly with a hexdump of the
>> failed message as well) and not forward the message to the driver. Not sure of
>> any better way of handling it then that
> Yeah I think this reflects what I wanted to do, I've no memory of a
> retransmit option in the spec, but I've away from it for a while. But
> we'd want to compare the CRC with what we got to make sure the are the
> same.

Hmm, that far more complex than just fix compilation warnings :)

I will split the patch in two:

- one for of all other warnings, hopefully it can get reviewed

- one for this crc4 variable. Does checking crc value and print an error 
should be acceptable ?

Something like:

if (crc4 != msg->chunk[msg->curchunk_len - 1])

     print_hex_dump(KERN_DEBUG, "wrong crc", DUMP_PREFIX_NONE, 16, 1, 
msg->chunk,  msg->curchunk_len, false);


Benjamin


>
> Dave.