Date:   Tue, 11 Jan 2022 09:39:13 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Jiasheng Jiang <jiasheng@iscas.ac.cn>
Cc:     heikki.krogerus@linux.intel.com, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: typec: Check error number after calling
 ida_simple_get
Message-ID: <Yd1CMdAzIXaT/QBB@kroah.com>
References: <20220111071751.590487-1-jiasheng@iscas.ac.cn>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220111071751.590487-1-jiasheng@iscas.ac.cn>
Precedence: bulk

On Tue, Jan 11, 2022 at 03:17:51PM +0800, Jiasheng Jiang wrote:
> If allocation fails, the ida_simple_get() will return error number.

How can allocation fail?  Have you been able to trigger this?

> So altmode_id_get() may return error number.
> And then id will be used in altmode_id_remove, causing the BUG_ON().
> Or it will be assigned to alt->id.
> Therefore, it should be better to check it and return error if fails,
> like the ida_simple_get() in typec_register_port().
> 
> Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  drivers/usb/typec/class.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index aeef453aa658..67b3670ede99 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -516,6 +516,9 @@ typec_register_altmode(struct device *parent,
>  	struct altmode *alt;
>  	int ret;
>  
> +	if (id < 0)
> +		return ERR_PTR(id);
> +
>  	alt = kzalloc(sizeof(*alt), GFP_KERNEL);
>  	if (!alt) {
>  		altmode_id_remove(parent, id);
> -- 
> 2.25.1
> 

How did you test that this change will work properly?

As I have said before, I'm not going to take changes from you unless you
can prove they have been tested, due to all of the previous problems in
your submitted patches.

greg k-h