lkml.org 
[lkml]   [2021]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRE: [PATCH] usb: gadget: composite: Fix null pointer exception
    Date
    > -----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
    \
     
     \ /
      Last update: 2021-11-02 04:59    [W:2.347 / U:0.704 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site