linux-kernel - Re: [PATCH RESEND 1/2] usb: typec: tipd: fix event checking for tps25750

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: <ZgvcOXtK15ggpKRE@kuha.fi.intel.com>
Date: Tue, 2 Apr 2024 13:21:45 +0300
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Javier Carrasco <javier.carrasco@...fvision.net>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Abdel Alkuor <abdelalkuor@...tab.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH RESEND 1/2] usb: typec: tipd: fix event checking for
 tps25750

On Thu, Mar 28, 2024 at 05:55:51PM +0100, Javier Carrasco wrote:
> In its current form, the interrupt service routine of the tps25750
> checks the event flags in the lowest 64 bits of the interrupt event
> register (event[0]), but also in the upper part (event[1]).
> 
> Given that all flags are defined as BIT() or BIT_ULL(), they are
> restricted to the first 64 bits of the INT_EVENT1 register. Including
> the upper part of the register can lead to false positives e.g. if the
> event 64 bits above the one being checked is set, but the one being
> checked is not.
> 
> Restrict the flag checking to the first 64 bits of the INT_EVENT1
> register.
> 
> Fixes: 7e7a3c815d22 ("USB: typec: tps6598x: Add TPS25750 support")
> Cc: stable@...r.kernel.org
> Signed-off-by: Javier Carrasco <javier.carrasco@...fvision.net>

Acked-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>

> ---
>  drivers/usb/typec/tipd/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 0717cfcd9f8c..7c2f01344860 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -604,11 +604,11 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
>  	if (!tps6598x_read_status(tps, &status))
>  		goto err_clear_ints;
>  
> -	if ((event[0] | event[1]) & TPS_REG_INT_POWER_STATUS_UPDATE)
> +	if (event[0] & TPS_REG_INT_POWER_STATUS_UPDATE)
>  		if (!tps6598x_read_power_status(tps))
>  			goto err_clear_ints;
>  
> -	if ((event[0] | event[1]) & TPS_REG_INT_DATA_STATUS_UPDATE)
> +	if (event[0] & TPS_REG_INT_DATA_STATUS_UPDATE)
>  		if (!tps6598x_read_data_status(tps))
>  			goto err_clear_ints;
>  
> @@ -617,7 +617,7 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
>  	 * a plug event. Therefore, we need to check
>  	 * for pr/dr status change to set TypeC dr/pr accordingly.
>  	 */
> -	if ((event[0] | event[1]) & TPS_REG_INT_PLUG_EVENT ||
> +	if (event[0] & TPS_REG_INT_PLUG_EVENT ||
>  	    tps6598x_has_role_changed(tps, status))
>  		tps6598x_handle_plug_event(tps, status);
>  
> 
> -- 
> 2.40.1

-- 
heikki