linux-kernel - Re: [PATCH] usb: typec: tcpm: Add IS_ERR_OR

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: <ZMeVnbnIuf1bNuKQ@kuha.fi.intel.com>
Date:   Mon, 31 Jul 2023 14:06:05 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Jimmy Hu <hhhuuu@...gle.com>
Cc:     linux@...ck-us.net, gregkh@...uxfoundation.org, kyletso@...gle.com,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: typec: tcpm: Add IS_ERR_OR_NULL check for
 port->partner

Hi,

I'm sorry to keep you waiting.

On Fri, Jun 30, 2023 at 06:57:11AM +0000, Jimmy Hu wrote:
> port->partner may be an error or NULL, so we must check it with
> IS_ERR_OR_NULL() before dereferencing it.

Have you seen this happening? Maybe the partner check should happen
earlier, before tcpm_pd_svdm() is even called?

> Fixes: 5e1d4c49fbc8 ("usb: typec: tcpm: Determine common SVDM Version")
> Signed-off-by: Jimmy Hu <hhhuuu@...gle.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 829d75ebab42..cd2590eead04 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1626,6 +1626,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  				break;
>  
>  			if (PD_VDO_SVDM_VER(p[0]) < svdm_version) {
> +				if (IS_ERR_OR_NULL(port->partner))
> +					break;
>  				typec_partner_set_svdm_version(port->partner,
>  							       PD_VDO_SVDM_VER(p[0]));
>  				svdm_version = PD_VDO_SVDM_VER(p[0]);

Now you will still build a response? I'm pretty sure you don't want
that.

Do we need to do anything in this function if the partner is lost? If
not, then why not just check the partner in the beginning of the
function. Or just make sure we don't even call tcpm_pd_svdm() if
there's no partner.

thanks,

-- 
heikki