[PATCH] =3D?UTF-8?q?usb:=3D20function:=3D20make=3D20sure=3D20usb= =3D?UTF-8?q?nction*=3D20functions=3D20don=3DE2=3D80=3D99t=3D20returen=3D20=

[Michal Nazarewicz]

The interface for usb_get_function and usb_get_function_instance is
that they return a pointer or an error pointer.  In other words, they
never return NULL.  For that to work, they rely on alloc_func and
alloc_inst callbacks respectively to never return NULL either.  Indeed,
if those callbacks ever return NULL, a null pointer dereference will
happen.

Be defensive about it and check in those functions whether the callbacks
(incorrectly) returned NULL and interpret it as ENOMEM error pointer
instead.

Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
---
 drivers/usb/gadget/functions.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/functions.c b/drivers/usb/gadget/functions.c
index b13f839..4a1f1fb 100644
--- a/drivers/usb/gadget/functions.c
+++ b/drivers/usb/gadget/functions.c
@@ -27,12 +27,12 @@ static struct usb_function_instance *try_get_usb_functi=
on_instance(const char *n
 		fi =3D fd->alloc_inst();
 		if (IS_ERR(fi))
 			module_put(fd->mod);
-		else
+		else if (fi)  /* This should always be true but be defensive. */
 			fi->fd =3D fd;
 		break;
 	}
 	mutex_unlock(&func_lock);
-	return fi;
+	return fi ? fi : ERR_PTR(-ENOMEM);
 }
=20
 struct usb_function_instance *usb_get_function_instance(const char *name)
@@ -43,6 +43,8 @@ struct usb_function_instance *usb_get_function_instance(c=
onst char *name)
 	fi =3D try_get_usb_function_instance(name);
 	if (!IS_ERR(fi))
 		return fi;
+	else if (!fi)  /* We should never be here but be defensive about it. */
+		return ERR_PTR(-ENOMEM);
 	ret =3D PTR_ERR(fi);
 	if (ret !=3D -ENOENT)
 		return fi;
@@ -60,6 +62,8 @@ struct usb_function *usb_get_function(struct usb_function=
_instance *fi)
 	f =3D fi->fd->alloc_func(fi);
 	if (IS_ERR(f))
 		return f;
+	else if (!f)  /* We should never be here but be defensive about it. */
+		return ERR_PTR(-ENOMEM);
 	f->fi =3D fi;
 	return f;
 }
--=20
2.8.0.rc3.226.g39d4020
----------- >8 ---------------------------------------------------------

However, I don=E2=80=99t think it is necessary and it only complicates code.

> ---
>  drivers/usb/gadget/legacy/nokia.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legac=
y/nokia.c
> index b1e535f..d3ba286 100644
> --- a/drivers/usb/gadget/legacy/nokia.c
> +++ b/drivers/usb/gadget/legacy/nokia.c
> @@ -159,36 +159,36 @@ static int nokia_bind_config(struct usb_configurati=
on *c)
>=20=20
>  	if (!IS_ERR(fi_phonet)) {
>  		f_phonet =3D usb_get_function(fi_phonet);
> -		if (IS_ERR(f_phonet))
> +		if (IS_ERR_OR_NULL(f_phonet))
>  			pr_debug("could not get phonet function\n");
>  	}
>=20=20
>  	if (!IS_ERR(fi_obex1)) {
>  		f_obex1 =3D usb_get_function(fi_obex1);
> -		if (IS_ERR(f_obex1))
> +		if (IS_ERR_OR_NULL(f_obex1))
>  			pr_debug("could not get obex function 0\n");
>  	}
>=20=20
>  	if (!IS_ERR(fi_obex2)) {
>  		f_obex2 =3D usb_get_function(fi_obex2);
> -		if (IS_ERR(f_obex2))
> +		if (IS_ERR_OR_NULL(f_obex2))
>  			pr_debug("could not get obex function 1\n");
>  	}
>=20=20
>  	f_acm =3D usb_get_function(fi_acm);
> -	if (IS_ERR(f_acm)) {
> +	if (IS_ERR_OR_NULL(f_acm)) {
>  		status =3D PTR_ERR(f_acm);
>  		goto err_get_acm;
>  	}
>=20=20
>  	f_ecm =3D usb_get_function(fi_ecm);
> -	if (IS_ERR(f_ecm)) {
> +	if (IS_ERR_OR_NULL(f_ecm)) {
>  		status =3D PTR_ERR(f_ecm);
>  		goto err_get_ecm;
>  	}
>=20=20
>  	f_msg =3D usb_get_function(fi_msg);
> -	if (IS_ERR(f_msg)) {
> +	if (IS_ERR_OR_NULL(f_msg)) {
>  		status =3D PTR_ERR(f_msg);
>  		goto err_get_msg;
>  	}
> --=20
> 2.7.4
>

--=20
Best regards
=E3=83=9F=E3=83=8F=E3=82=A6 =E2=80=9C=F0=9D=93=B6=F0=9D=93=B2=F0=9D=93=B7=
=F0=9D=93=AA86=E2=80=9D =E3=83=8A=E3=82=B6=E3=83=AC=E3=83=B4=E3=82=A4=E3=83=
=84
=C2=ABIf at first you don=E2=80=99t succeed, give up skydiving=C2=BB