linux-kernel - Re: [PATCH v3 3/7] usb: typec: Check port is active before enter mode on probe

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: <2024110849-bonded-flatten-5f5e@gregkh>
Date: Fri, 8 Nov 2024 14:17:23 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
Cc: heikki.krogerus@...ux.intel.com, tzungbi@...nel.org,
	linux-usb@...r.kernel.org, chrome-platform@...ts.linux.dev,
	jthies@...gle.com, akuchynski@...gle.com, pmalani@...omium.org,
	dmitry.baryshkov@...aro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/7] usb: typec: Check port is active before enter
 mode on probe

On Thu, Nov 07, 2024 at 11:29:56AM -0800, Abhishek Pandit-Subedi wrote:
> Enforce the same requirement as when we attempt to activate altmode via
> sysfs (do not enter partner mode if port mode is not active). In order
> to set a default value for this field, also introduce the inactive field
> in struct typec_altmode_desc.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
> ---
> 
> Changes in v3:
> - Use port.active instead of introducing auto-enter field
> - Introduce inactive field to typec_altmode_desc to set default value
>   for active.
> - Always make port 'active' field writable
> 
>  drivers/usb/typec/altmodes/displayport.c | 7 +++++--
>  drivers/usb/typec/altmodes/thunderbolt.c | 6 +++++-
>  drivers/usb/typec/class.c                | 5 +++--
>  include/linux/usb/typec.h                | 2 ++
>  4 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 3245e03d59e6..f4116e96f6a1 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -767,8 +767,11 @@ int dp_altmode_probe(struct typec_altmode *alt)
>  	if (plug)
>  		typec_altmode_set_drvdata(plug, dp);
>  
> -	dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
> -	schedule_work(&dp->work);
> +	/* Only attempt to enter altmode if port is active. */
> +	if (port->active) {
> +		dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
> +		schedule_work(&dp->work);
> +	}

What prevents active from changing right after you checked it?

> @@ -150,6 +151,7 @@ struct typec_altmode_desc {
>  	u32			vdo;
>  	/* Only used with ports */
>  	enum typec_port_data	roles;
> +	bool			inactive : 1;

A boolean bitfield?  That's not needed, please just do this properly.

thanks,

greg k-h