linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: composite: Fix null pointer exception
@ 2021-11-01  1:57 Qihang Hu
  2021-11-01 13:18 ` Peter Chen
  0 siblings, 1 reply; 5+ messages in thread
From: Qihang Hu @ 2021-11-01  1:57 UTC (permalink / raw)
  To: balbi, gregkh; +Cc: linux-usb, linux-kernel, Qihang Hu

In the config_ep_by_speed_and_alt function, select the corresponding
descriptor through g->speed, but the interface driver may not
support the corresponding speed. So, we need to check whether the
interface driver provides the corresponding speed descriptor when
selecting the descriptor.

[  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
[  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
[  237.712487]  kUnable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[  237.712493]  kMem abort info:
[  237.712498]  k  ESR = 0x96000006
[  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
[  237.712510]  k  SET = 0, FnV = 0
[  237.712515]  k  EA = 0, S1PTW = 0
[  237.712520]  kData abort info:
[  237.712525]  k  ISV = 0, ISS = 0x00000006
[  237.712530]  k  CM = 0, WnR = 0
[  237.712536]  kuser pgtable: 4k pages, 39-bit VAs, pgdp=000000020ef29000
[  237.712541]  k[0000000000000000] pgd=000000020ef2a003, pud=000000020ef2a003, pmd=0000000000000000
[  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
[  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
[  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
[  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
[  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
[  237.871766]  klr : audio_set_alt+0x54/0x78
[  237.879108]  ksp : ffffffc0104839e0

Signed-off-by: Qihang Hu <huqihang@oppo.com>
---
 drivers/usb/gadget/composite.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 72a9797dbbae..443a65af98af 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -166,21 +166,21 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
 	/* select desired speed */
 	switch (g->speed) {
 	case USB_SPEED_SUPER_PLUS:
-		if (gadget_is_superspeed_plus(g)) {
+		if (gadget_is_superspeed_plus(g) && f->ssp_descriptors) {
 			speed_desc = f->ssp_descriptors;
 			want_comp_desc = 1;
 			break;
 		}
 		fallthrough;
 	case USB_SPEED_SUPER:
-		if (gadget_is_superspeed(g)) {
+		if (gadget_is_superspeed(g) && f->ss_descriptors) {
 			speed_desc = f->ss_descriptors;
 			want_comp_desc = 1;
 			break;
 		}
 		fallthrough;
 	case USB_SPEED_HIGH:
-		if (gadget_is_dualspeed(g)) {
+		if (gadget_is_dualspeed(g) && f->hs_descriptors) {
 			speed_desc = f->hs_descriptors;
 			break;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: gadget: composite: Fix null pointer exception
  2021-11-01  1:57 [PATCH] usb: gadget: composite: Fix null pointer exception Qihang Hu
@ 2021-11-01 13:18 ` Peter Chen
  2021-11-02  3:59   ` 胡启航(Nick Hu)
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Chen @ 2021-11-01 13:18 UTC (permalink / raw)
  To: Qihang Hu; +Cc: balbi, gregkh, linux-usb, linux-kernel

On 21-11-01 09:57:57, Qihang Hu wrote:
> In the config_ep_by_speed_and_alt function, select the corresponding
> descriptor through g->speed, but the interface driver

function driver

> may not
> support the corresponding speed. So, we need to check whether the
> interface driver provides the corresponding speed descriptor when
> selecting the descriptor.
> 
> [  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
> [  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
> [  237.712487]  kUnable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [  237.712493]  kMem abort info:
> [  237.712498]  k  ESR = 0x96000006
> [  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
> [  237.712510]  k  SET = 0, FnV = 0
> [  237.712515]  k  EA = 0, S1PTW = 0
> [  237.712520]  kData abort info:
> [  237.712525]  k  ISV = 0, ISS = 0x00000006
> [  237.712530]  k  CM = 0, WnR = 0
> [  237.712536]  kuser pgtable: 4k pages, 39-bit VAs, pgdp=000000020ef29000
> [  237.712541]  k[0000000000000000] pgd=000000020ef2a003, pud=000000020ef2a003, pmd=0000000000000000
> [  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
> [  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
> [  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
> [  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
> [  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
> [  237.871766]  klr : audio_set_alt+0x54/0x78
> [  237.879108]  ksp : ffffffc0104839e0
> 
> Signed-off-by: Qihang Hu <huqihang@oppo.com>
> ---
>  drivers/usb/gadget/composite.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 72a9797dbbae..443a65af98af 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -166,21 +166,21 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
>  	/* select desired speed */
>  	switch (g->speed) {
>  	case USB_SPEED_SUPER_PLUS:
> -		if (gadget_is_superspeed_plus(g)) {
> +		if (gadget_is_superspeed_plus(g) && f->ssp_descriptors) {
>  			speed_desc = f->ssp_descriptors;
>  			want_comp_desc = 1;
>  			break;
>  		}
>  		fallthrough;
>  	case USB_SPEED_SUPER:
> -		if (gadget_is_superspeed(g)) {
> +		if (gadget_is_superspeed(g) && f->ss_descriptors) {
>  			speed_desc = f->ss_descriptors;
>  			want_comp_desc = 1;
>  			break;
>  		}
>  		fallthrough;
>  	case USB_SPEED_HIGH:
> -		if (gadget_is_dualspeed(g)) {
> +		if (gadget_is_dualspeed(g) && f->hs_descriptors) {
>  			speed_desc = f->hs_descriptors;
>  			break;
>  		}
> -- 
> 2.25.1
> 

Besides your fix, you may show an warning that said "the function
doesn't hold the descriptors for supported speed, using the default (FS)
descriptors". See below kernel doc for detail.

/**
 * config_ep_by_speed_and_alt() - configures the given endpoint
 *
 * ....
 * Note: the supplied function should hold all the descriptors
 * for supported speeds
 */

What's more, you may fix android f_audio_source.c, and let it support
super speed and super speed plus.

-- 

Thanks,
Peter Chen


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] usb: gadget: composite: Fix null pointer exception
  2021-11-01 13:18 ` Peter Chen
@ 2021-11-02  3:59   ` 胡启航(Nick Hu)
  2021-11-04 12:18     ` Peter Chen
  2021-11-05  9:58     ` gregkh
  0 siblings, 2 replies; 5+ messages in thread
From: 胡启航(Nick Hu) @ 2021-11-02  3:59 UTC (permalink / raw)
  To: Peter Chen; +Cc: balbi, gregkh, linux-usb, linux-kernel

> -----Original Message-----
> From: Peter Chen <peter.chen@kernel.org>
> Sent: Monday, November 1, 2021 9:19 PM
> To: 胡启航(Nick Hu) <huqihang@oppo.com>
> Cc: balbi@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] usb: gadget: composite: Fix null pointer exception
> 
> On 21-11-01 09:57:57, Qihang Hu wrote:
> > In the config_ep_by_speed_and_alt function, select the corresponding
> > descriptor through g->speed, but the interface driver
> 
> function driver
> 
> > may not
> > support the corresponding speed. So, we need to check whether the
> > interface driver provides the corresponding speed descriptor when
> > selecting the descriptor.
> >
> > [  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
> > [  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
> > [  237.712487]  kUnable to handle kernel NULL pointer dereference at
> virtual address 0000000000000000
> > [  237.712493]  kMem abort info:
> > [  237.712498]  k  ESR = 0x96000006
> > [  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
> > [  237.712510]  k  SET = 0, FnV = 0
> > [  237.712515]  k  EA = 0, S1PTW = 0
> > [  237.712520]  kData abort info:
> > [  237.712525]  k  ISV = 0, ISS = 0x00000006
> > [  237.712530]  k  CM = 0, WnR = 0
> > [  237.712536]  kuser pgtable: 4k pages, 39-bit VAs,
> pgdp=000000020ef29000
> > [  237.712541]  k[0000000000000000] pgd=000000020ef2a003,
> pud=000000020ef2a003, pmd=0000000000000000
> > [  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
> > [  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
> > [  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
> > [  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
> > [  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
> > [  237.871766]  klr : audio_set_alt+0x54/0x78
> > [  237.879108]  ksp : ffffffc0104839e0
> >
> > Signed-off-by: Qihang Hu <huqihang@oppo.com>
> > ---
> >  drivers/usb/gadget/composite.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > index 72a9797dbbae..443a65af98af 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -166,21 +166,21 @@ int config_ep_by_speed_and_alt(struct usb_gadget
> *g,
> >  	/* select desired speed */
> >  	switch (g->speed) {
> >  	case USB_SPEED_SUPER_PLUS:
> > -		if (gadget_is_superspeed_plus(g)) {
> > +		if (gadget_is_superspeed_plus(g) && f->ssp_descriptors) {
> >  			speed_desc = f->ssp_descriptors;
> >  			want_comp_desc = 1;
> >  			break;
> >  		}
> >  		fallthrough;
> >  	case USB_SPEED_SUPER:
> > -		if (gadget_is_superspeed(g)) {
> > +		if (gadget_is_superspeed(g) && f->ss_descriptors) {
> >  			speed_desc = f->ss_descriptors;
> >  			want_comp_desc = 1;
> >  			break;
> >  		}
> >  		fallthrough;
> >  	case USB_SPEED_HIGH:
> > -		if (gadget_is_dualspeed(g)) {
> > +		if (gadget_is_dualspeed(g) && f->hs_descriptors) {
> >  			speed_desc = f->hs_descriptors;
> >  			break;
> >  		}
> > --
> > 2.25.1
> >
> 
> Besides your fix, you may show an warning that said "the function
> doesn't hold the descriptors for supported speed, using the default (FS)
> descriptors". See below kernel doc for detail.
> 
> /**
>  * config_ep_by_speed_and_alt() - configures the given endpoint
>  *
>  * ....
>  * Note: the supplied function should hold all the descriptors
>  * for supported speeds
>  */
> 
> What's more, you may fix android f_audio_source.c, and let it support
> super speed and super speed plus.
> 
> --
> 
> Thanks,
> Peter Chen



From 9b8262792b6e85e6060601dbfc651b1e75b649f0 Mon Sep 17 00:00:00 2001
From: Qihang Hu <huqihang@oppo.com>
Date: Sat, 30 Oct 2021 16:11:38 +0800
Subject: [PATCH] usb: gadget: composite: Fix null pointer exception

In the config_ep_by_speed_and_alt function, select the corresponding
descriptor through g->speed, but the function driver may not
support the corresponding speed. So, we need to check whether the
function driver provides the corresponding speed descriptor when
selecting the descriptor.

[  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
[  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
[  237.712487]  kUnable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[  237.712493]  kMem abort info:
[  237.712498]  k  ESR = 0x96000006
[  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
[  237.712510]  k  SET = 0, FnV = 0
[  237.712515]  k  EA = 0, S1PTW = 0
[  237.712520]  kData abort info:
[  237.712525]  k  ISV = 0, ISS = 0x00000006
[  237.712530]  k  CM = 0, WnR = 0
[  237.712536]  kuser pgtable: 4k pages, 39-bit VAs, pgdp=000000020ef29000
[  237.712541]  k[0000000000000000] pgd=000000020ef2a003, pud=000000020ef2a003, pmd=0000000000000000
[  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
[  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
[  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
[  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
[  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
[  237.871766]  klr : audio_set_alt+0x54/0x78
[  237.879108]  ksp : ffffffc0104839e0

Signed-off-by: Qihang Hu <huqihang@oppo.com>
---
 drivers/usb/gadget/composite.c | 39 ++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 72a9797dbbae..4f0d81f561ae 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -151,9 +151,11 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
 				struct usb_ep *_ep,
 				u8 alt)
 {
+	struct usb_composite_dev *cdev;
 	struct usb_endpoint_descriptor *chosen_desc = NULL;
 	struct usb_interface_descriptor *int_desc = NULL;
 	struct usb_descriptor_header **speed_desc = NULL;
+	int incomplete_desc = 0;
 
 	struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
 	int want_comp_desc = 0;
@@ -167,28 +169,43 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
 	switch (g->speed) {
 	case USB_SPEED_SUPER_PLUS:
 		if (gadget_is_superspeed_plus(g)) {
-			speed_desc = f->ssp_descriptors;
-			want_comp_desc = 1;
-			break;
+			if (f->ssp_descriptors) {
+				speed_desc = f->ssp_descriptors;
+				want_comp_desc = 1;
+				break;
+			}
+			incomplete_desc = 1;
 		}
 		fallthrough;
 	case USB_SPEED_SUPER:
 		if (gadget_is_superspeed(g)) {
-			speed_desc = f->ss_descriptors;
-			want_comp_desc = 1;
-			break;
+			if (f->ss_descriptors) {
+				speed_desc = f->ss_descriptors;
+				want_comp_desc = 1;
+				break;
+			}
+			incomplete_desc = 1;
 		}
 		fallthrough;
 	case USB_SPEED_HIGH:
 		if (gadget_is_dualspeed(g)) {
-			speed_desc = f->hs_descriptors;
-			break;
+			if (f->hs_descriptors) {
+				speed_desc = f->hs_descriptors;
+				break;
+			}
+			incomplete_desc = 1;
 		}
 		fallthrough;
 	default:
 		speed_desc = f->fs_descriptors;
 	}
 
+	cdev = get_gadget_data(g);
+	if (incomplete_desc != 0)
+		WARNING(cdev,
+			"%s function doesn't hold all the descriptors for supported speep\n",
+			f->name);
+
 	/* find correct alternate setting descriptor */
 	for_each_desc(speed_desc, d_spd, USB_DT_INTERFACE) {
 		int_desc = (struct usb_interface_descriptor *)*d_spd;
@@ -244,12 +261,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
 			_ep->maxburst = comp_desc->bMaxBurst + 1;
 			break;
 		default:
-			if (comp_desc->bMaxBurst != 0) {
-				struct usb_composite_dev *cdev;
-
-				cdev = get_gadget_data(g);
+			if (comp_desc->bMaxBurst != 0)
 				ERROR(cdev, "ep0 bMaxBurst must be 0\n");
-			}
 			_ep->maxburst = 1;
 			break;
 		}
-- 
2.25.1


Thanks for your suggestion, this is my revised patch.
Of course, I will continue to fix android f_audio_source.c to solve the problem fundamentally.

Thanks

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: gadget: composite: Fix null pointer exception
  2021-11-02  3:59   ` 胡启航(Nick Hu)
@ 2021-11-04 12:18     ` Peter Chen
  2021-11-05  9:58     ` gregkh
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Chen @ 2021-11-04 12:18 UTC (permalink / raw)
  To: 胡启航(Nick Hu); +Cc: balbi, gregkh, linux-usb, linux-kernel

On 21-11-02 03:59:16, 胡启航(Nick Hu) wrote:
> From 9b8262792b6e85e6060601dbfc651b1e75b649f0 Mon Sep 17 00:00:00 2001
> From: Qihang Hu <huqihang@oppo.com>
> Date: Sat, 30 Oct 2021 16:11:38 +0800
> Subject: [PATCH] usb: gadget: composite: Fix null pointer exception
> 
> In the config_ep_by_speed_and_alt function, select the corresponding
> descriptor through g->speed, but the function driver may not
> support the corresponding speed. So, we need to check whether the
> function driver provides the corresponding speed descriptor when
> selecting the descriptor.
> 
> [  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
> [  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
> [  237.712487]  kUnable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [  237.712493]  kMem abort info:
> [  237.712498]  k  ESR = 0x96000006
> [  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
> [  237.712510]  k  SET = 0, FnV = 0
> [  237.712515]  k  EA = 0, S1PTW = 0
> [  237.712520]  kData abort info:
> [  237.712525]  k  ISV = 0, ISS = 0x00000006
> [  237.712530]  k  CM = 0, WnR = 0
> [  237.712536]  kuser pgtable: 4k pages, 39-bit VAs, pgdp=000000020ef29000
> [  237.712541]  k[0000000000000000] pgd=000000020ef2a003, pud=000000020ef2a003, pmd=0000000000000000
> [  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
> [  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
> [  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
> [  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
> [  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
> [  237.871766]  klr : audio_set_alt+0x54/0x78
> [  237.879108]  ksp : ffffffc0104839e0
> 
> Signed-off-by: Qihang Hu <huqihang@oppo.com>
> ---
>  drivers/usb/gadget/composite.c | 39 ++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 72a9797dbbae..4f0d81f561ae 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -151,9 +151,11 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
>  				struct usb_ep *_ep,
>  				u8 alt)
>  {
> +	struct usb_composite_dev *cdev;

For newly added variable definition, you could add it at the last existing
variable definition, see position "..." below.

>  	struct usb_endpoint_descriptor *chosen_desc = NULL;
>  	struct usb_interface_descriptor *int_desc = NULL;
>  	struct usb_descriptor_header **speed_desc = NULL;
...
> +	int incomplete_desc = 0;
>  
>  	struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
>  	int want_comp_desc = 0;
> @@ -167,28 +169,43 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
>  	switch (g->speed) {
>  	case USB_SPEED_SUPER_PLUS:
>  		if (gadget_is_superspeed_plus(g)) {
> -			speed_desc = f->ssp_descriptors;
> -			want_comp_desc = 1;
> -			break;
> +			if (f->ssp_descriptors) {
> +				speed_desc = f->ssp_descriptors;
> +				want_comp_desc = 1;
> +				break;
> +			}
> +			incomplete_desc = 1;
>  		}
>  		fallthrough;
>  	case USB_SPEED_SUPER:
>  		if (gadget_is_superspeed(g)) {
> -			speed_desc = f->ss_descriptors;
> -			want_comp_desc = 1;
> -			break;
> +			if (f->ss_descriptors) {
> +				speed_desc = f->ss_descriptors;
> +				want_comp_desc = 1;
> +				break;
> +			}
> +			incomplete_desc = 1;
>  		}
>  		fallthrough;
>  	case USB_SPEED_HIGH:
>  		if (gadget_is_dualspeed(g)) {
> -			speed_desc = f->hs_descriptors;
> -			break;
> +			if (f->hs_descriptors) {
> +				speed_desc = f->hs_descriptors;
> +				break;
> +			}
> +			incomplete_desc = 1;
>  		}
>  		fallthrough;
>  	default:
>  		speed_desc = f->fs_descriptors;
>  	}
>  
> +	cdev = get_gadget_data(g);
> +	if (incomplete_desc != 0)
> +		WARNING(cdev,
> +			"%s function doesn't hold all the descriptors for supported speep\n",

"%s doesn't hold the descriptors for current speed"

> +			f->name);
> +
>  	/* find correct alternate setting descriptor */
>  	for_each_desc(speed_desc, d_spd, USB_DT_INTERFACE) {
>  		int_desc = (struct usb_interface_descriptor *)*d_spd;
> @@ -244,12 +261,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
>  			_ep->maxburst = comp_desc->bMaxBurst + 1;
>  			break;
>  		default:
> -			if (comp_desc->bMaxBurst != 0) {
> -				struct usb_composite_dev *cdev;
> -
> -				cdev = get_gadget_data(g);
> +			if (comp_desc->bMaxBurst != 0)
>  				ERROR(cdev, "ep0 bMaxBurst must be 0\n");
> -			}
>  			_ep->maxburst = 1;
>  			break;
>  		}
> -- 
> 2.25.1
> 
> 
> Thanks for your suggestion, this is my revised patch.
> Of course, I will continue to fix android f_audio_source.c to solve the problem fundamentally.
> 
> Thanks

Other things are ok for me. You could send v2 version after addressing my comments.

-- 

Thanks,
Peter Chen


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: gadget: composite: Fix null pointer exception
  2021-11-02  3:59   ` 胡启航(Nick Hu)
  2021-11-04 12:18     ` Peter Chen
@ 2021-11-05  9:58     ` gregkh
  1 sibling, 0 replies; 5+ messages in thread
From: gregkh @ 2021-11-05  9:58 UTC (permalink / raw)
  To: 胡启航(Nick Hu)
  Cc: Peter Chen, balbi, linux-usb, linux-kernel

On Tue, Nov 02, 2021 at 03:59:16AM +0000, 胡启航(Nick Hu) wrote:
> > -----Original Message-----
> > From: Peter Chen <peter.chen@kernel.org>
> > Sent: Monday, November 1, 2021 9:19 PM
> > To: 胡启航(Nick Hu) <huqihang@oppo.com>
> > Cc: balbi@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] usb: gadget: composite: Fix null pointer exception
> > 
> > On 21-11-01 09:57:57, Qihang Hu wrote:
> > > In the config_ep_by_speed_and_alt function, select the corresponding
> > > descriptor through g->speed, but the interface driver
> > 
> > function driver
> > 
> > > may not
> > > support the corresponding speed. So, we need to check whether the
> > > interface driver provides the corresponding speed descriptor when
> > > selecting the descriptor.
> > >
> > > [  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
> > > [  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
> > > [  237.712487]  kUnable to handle kernel NULL pointer dereference at
> > virtual address 0000000000000000
> > > [  237.712493]  kMem abort info:
> > > [  237.712498]  k  ESR = 0x96000006
> > > [  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
> > > [  237.712510]  k  SET = 0, FnV = 0
> > > [  237.712515]  k  EA = 0, S1PTW = 0
> > > [  237.712520]  kData abort info:
> > > [  237.712525]  k  ISV = 0, ISS = 0x00000006
> > > [  237.712530]  k  CM = 0, WnR = 0
> > > [  237.712536]  kuser pgtable: 4k pages, 39-bit VAs,
> > pgdp=000000020ef29000
> > > [  237.712541]  k[0000000000000000] pgd=000000020ef2a003,
> > pud=000000020ef2a003, pmd=0000000000000000
> > > [  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
> > > [  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
> > > [  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
> > > [  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
> > > [  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
> > > [  237.871766]  klr : audio_set_alt+0x54/0x78
> > > [  237.879108]  ksp : ffffffc0104839e0
> > >
> > > Signed-off-by: Qihang Hu <huqihang@oppo.com>
> > > ---
> > >  drivers/usb/gadget/composite.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > > index 72a9797dbbae..443a65af98af 100644
> > > --- a/drivers/usb/gadget/composite.c
> > > +++ b/drivers/usb/gadget/composite.c
> > > @@ -166,21 +166,21 @@ int config_ep_by_speed_and_alt(struct usb_gadget
> > *g,
> > >  	/* select desired speed */
> > >  	switch (g->speed) {
> > >  	case USB_SPEED_SUPER_PLUS:
> > > -		if (gadget_is_superspeed_plus(g)) {
> > > +		if (gadget_is_superspeed_plus(g) && f->ssp_descriptors) {
> > >  			speed_desc = f->ssp_descriptors;
> > >  			want_comp_desc = 1;
> > >  			break;
> > >  		}
> > >  		fallthrough;
> > >  	case USB_SPEED_SUPER:
> > > -		if (gadget_is_superspeed(g)) {
> > > +		if (gadget_is_superspeed(g) && f->ss_descriptors) {
> > >  			speed_desc = f->ss_descriptors;
> > >  			want_comp_desc = 1;
> > >  			break;
> > >  		}
> > >  		fallthrough;
> > >  	case USB_SPEED_HIGH:
> > > -		if (gadget_is_dualspeed(g)) {
> > > +		if (gadget_is_dualspeed(g) && f->hs_descriptors) {
> > >  			speed_desc = f->hs_descriptors;
> > >  			break;
> > >  		}
> > > --
> > > 2.25.1
> > >
> > 
> > Besides your fix, you may show an warning that said "the function
> > doesn't hold the descriptors for supported speed, using the default (FS)
> > descriptors". See below kernel doc for detail.
> > 
> > /**
> >  * config_ep_by_speed_and_alt() - configures the given endpoint
> >  *
> >  * ....
> >  * Note: the supplied function should hold all the descriptors
> >  * for supported speeds
> >  */
> > 
> > What's more, you may fix android f_audio_source.c, and let it support
> > super speed and super speed plus.
> > 
> > --
> > 
> > Thanks,
> > Peter Chen
> 
> 
> 
> From 9b8262792b6e85e6060601dbfc651b1e75b649f0 Mon Sep 17 00:00:00 2001
> From: Qihang Hu <huqihang@oppo.com>
> Date: Sat, 30 Oct 2021 16:11:38 +0800
> Subject: [PATCH] usb: gadget: composite: Fix null pointer exception
> 
> In the config_ep_by_speed_and_alt function, select the corresponding
> descriptor through g->speed, but the function driver may not
> support the corresponding speed. So, we need to check whether the
> function driver provides the corresponding speed descriptor when
> selecting the descriptor.
> 
> [  237.708146]  android_work: sent uevent USB_STATE=CONNECTED
> [  237.712464]  kconfigfs-gadget gadget: super-speed config #1: b
> [  237.712487]  kUnable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [  237.712493]  kMem abort info:
> [  237.712498]  k  ESR = 0x96000006
> [  237.712504]  k  EC = 0x25: DABT (current EL), IL = 32 bits
> [  237.712510]  k  SET = 0, FnV = 0
> [  237.712515]  k  EA = 0, S1PTW = 0
> [  237.712520]  kData abort info:
> [  237.712525]  k  ISV = 0, ISS = 0x00000006
> [  237.712530]  k  CM = 0, WnR = 0
> [  237.712536]  kuser pgtable: 4k pages, 39-bit VAs, pgdp=000000020ef29000
> [  237.712541]  k[0000000000000000] pgd=000000020ef2a003, pud=000000020ef2a003, pmd=0000000000000000
> [  237.712554]  kInternal error: Oops: 96000006 [#1] PREEMPT SMP
> [  237.722067]  kSkip md ftrace buffer dump for: 0x1609e0
> [  237.787037]  kWorkqueue: dwc_wq dwc3_bh_work.cfi_jt
> [  237.854922]  kpstate: 60c00085 (nZCv daIf +PAN +UAO)
> [  237.863165]  kpc : config_ep_by_speed_and_alt+0x90/0x308
> [  237.871766]  klr : audio_set_alt+0x54/0x78
> [  237.879108]  ksp : ffffffc0104839e0
> 
> Signed-off-by: Qihang Hu <huqihang@oppo.com>
> ---
>  drivers/usb/gadget/composite.c | 39 ++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 13 deletions(-)

I can not take patches at the end of other email messages.

Please send this properly as a v2 patch, as the documentation asks for.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-05  9:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01  1:57 [PATCH] usb: gadget: composite: Fix null pointer exception Qihang Hu
2021-11-01 13:18 ` Peter Chen
2021-11-02  3:59   ` 胡启航(Nick Hu)
2021-11-04 12:18     ` Peter Chen
2021-11-05  9:58     ` gregkh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).