linux-kernel - Re: [PATCH v2] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains

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:   Sun, 2 Jul 2023 00:19:11 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Zhang Shurong <zhang_shurong@...mail.com>,
        gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] usb: r8a66597-hcd: host: fix port index underflow and
 UBSAN complains

Hello Alan,

On Sat, Jul 01, 2023 at 02:54:46PM -0400, Alan Stern wrote:
> wIndex should never be == 0 or > max_root_hub in the cases where rh gets 
> used; such values would be meaningless.  But we don't control the value 
> of wIndex, because it can come from userspace.  So we can't simply 
> assume it will always be valid; it has to be checked.
> 
> That being understood, the changes Zhang is making here are meant mostly 
> to prevent UBSAN and the compiler from complaining or making false 
> assumptions.  The actual checks on wIndex occur later in the subroutine.

I'm guilty of not having looked at all on that function, but it sounds
wrong to me to calculate values from some untrusted input and only
later validate the input. It should be the other way round, shouldn't
it? This is calling for compiler optimisations stepping on your toes.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)