netdev - RE: [Intel-wired-lan] Possible read-modify-write bug in ixgbe x550 phy setup

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]

Date:   Fri, 2 Feb 2018 21:08:25 +0000
From:   "Tantilov, Emil S" <emil.s.tantilov@...el.com>
To:     "Tantilov, Emil S" <emil.s.tantilov@...el.com>,
        Shannon Nelson <shannon.nelson@...cle.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "Greenwalt, Paul" <paul.greenwalt@...el.com>
Subject: RE: [Intel-wired-lan] Possible read-modify-write bug in ixgbe x550
 phy setup

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces@...osl.org] On
>Behalf Of Tantilov, Emil S
>Sent: Thursday, February 01, 2018 4:58 PM
>To: Shannon Nelson <shannon.nelson@...cle.com>
>Cc: netdev@...r.kernel.org; intel-wired-lan@...ts.osuosl.org
>Subject: Re: [Intel-wired-lan] Possible read-modify-write bug in ixgbe
>x550 phy setup
>
>>-----Original Message-----
>>From: Shannon Nelson [mailto:shannon.nelson@...cle.com]
>>Sent: Thursday, February 01, 2018 4:44 PM
>>To: Tantilov, Emil S <emil.s.tantilov@...el.com>
>>Cc: netdev@...r.kernel.org; intel-wired-lan@...ts.osuosl.org
>>Subject: Re: [Intel-wired-lan] Possible read-modify-write bug in ixgbe
>>x550 phy setup
>>
>>On 2/1/2018 4:34 PM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@...osl.org] On
>>>> Behalf Of Shannon Nelson
>>>> Sent: Thursday, February 01, 2018 3:46 PM
>>>> To: Tantilov, Emil S <emil.s.tantilov@...el.com>
>>>> Cc: netdev@...r.kernel.org; intel-wired-lan@...ts.osuosl.org
>>>> Subject: [Intel-wired-lan] Possible read-modify-write bug in ixgbe
>>x550
>>>> phy setup
>>>>
>>>> Hi Emil,
>>>>
>>>> I was looking through a set of ixgbe patches and came across this
>>commit
>>>>
>>>>      commit 410a494902777c11f95031d9ed757d7f8f09c5c6
>>>>      ixgbe: add write flush when configuring CS4223/7
>>>>
>>>> and am wondering about the setting of reg_phy_ext in the middle of
>>>> ixgbe_setup_mac_link_sfp_x550a().  It looks like it is read from the
>>>> PHY, modified to remove the CX1 and SR mode bits, but then those
>bits
>>>> are overwritten in the "if (setup_linear)" block immediately
>>following,
>>>> and that is what gets written back out.
>>>
>>> Hi Shannon,
>>>
>>> This is pretty standard clear before set, so you're right that it
>>would
>>> make more sense to have |= rather than =.
>>>
>>> Are you seeing an issue, or did you catch this via code inspection?
>>
>>Purely a code inspection, I saw this while looking at something else.
>>
>>sln
>
>Thanks for letting us know. I will do some more testing with the actual
>HW before changing the code.

Just FYI - we looked at the reads and confirmed that there is no functional
bug in the code because as it happens the CX1/SR bits is the only bits that
are read and set and as such we don't lose any data. This of course means
that the read is not needed and we'll just remove it.

Thanks,
Emil