| Message ID | 20210701234317.26393-6-digetx@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add OTG mode support to Tegra USB PHY, SMB347 and Nexus 7 | expand |
On 21-07-02 02:43:10, Dmitry Osipenko wrote: > The HNP work can be re-scheduled while it's still in-fly. This results in > re-initialization of the busy work, resetting the hrtimer's list node of > the work and crashing kernel with null dereference within kernel/timer > once work's timer is expired. It's very easy to trigger this problem by > re-plugging USB cable quickly. Initialize HNP work only once to fix this > trouble. Fully OTG compliance support has not maintained for years, what's the use case you still want to use? Peter > > Cc: stable@vger.kernel.org > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/usb/common/usb-otg-fsm.c | 6 +++++- > include/linux/usb/otg-fsm.h | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c > index 3740cf95560e..0697fde51d00 100644 > --- a/drivers/usb/common/usb-otg-fsm.c > +++ b/drivers/usb/common/usb-otg-fsm.c > @@ -193,7 +193,11 @@ static void otg_start_hnp_polling(struct otg_fsm *fsm) > if (!fsm->host_req_flag) > return; > > - INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work); > + if (!fsm->hnp_work_inited) { > + INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work); > + fsm->hnp_work_inited = true; > + } > + > schedule_delayed_work(&fsm->hnp_polling_work, > msecs_to_jiffies(T_HOST_REQ_POLL)); > } > diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h > index 3aee78dda16d..784659d4dc99 100644 > --- a/include/linux/usb/otg-fsm.h > +++ b/include/linux/usb/otg-fsm.h > @@ -196,6 +196,7 @@ struct otg_fsm { > struct mutex lock; > u8 *host_req_flag; > struct delayed_work hnp_polling_work; > + bool hnp_work_inited; > bool state_changed; > }; > > -- > 2.30.2 >
03.07.2021 14:08, Peter Chen пишет: > On 21-07-02 02:43:10, Dmitry Osipenko wrote: >> The HNP work can be re-scheduled while it's still in-fly. This results in >> re-initialization of the busy work, resetting the hrtimer's list node of >> the work and crashing kernel with null dereference within kernel/timer >> once work's timer is expired. It's very easy to trigger this problem by >> re-plugging USB cable quickly. Initialize HNP work only once to fix this >> trouble. > > Fully OTG compliance support has not maintained for years, what's the use case you > still want to use? I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it was crashing kernel badly. The OTG works perfectly fine without the FSM.
On 21-07-03 20:22:38, Dmitry Osipenko wrote: > 03.07.2021 14:08, Peter Chen пишет: > > On 21-07-02 02:43:10, Dmitry Osipenko wrote: > >> The HNP work can be re-scheduled while it's still in-fly. This results in > >> re-initialization of the busy work, resetting the hrtimer's list node of > >> the work and crashing kernel with null dereference within kernel/timer > >> once work's timer is expired. It's very easy to trigger this problem by > >> re-plugging USB cable quickly. Initialize HNP work only once to fix this > >> trouble. > > > > Fully OTG compliance support has not maintained for years, what's the use case you > > still want to use? > > I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it > was crashing kernel badly. The OTG works perfectly fine without the FSM. You could add below at your dts to disable OTG FSM: hnp-disable srp-disable adp-disable Since there are no users for OTG FSM, it hasn't maintained for years, I am not sure if it still works OK. If I remember correctly, the VBUS will be off if you enable HNP, and the device at the host port will be disconnected, that's may not your expectation.
05.07.2021 05:21, Peter Chen пишет: > On 21-07-03 20:22:38, Dmitry Osipenko wrote: >> 03.07.2021 14:08, Peter Chen пишет: >>> On 21-07-02 02:43:10, Dmitry Osipenko wrote: >>>> The HNP work can be re-scheduled while it's still in-fly. This results in >>>> re-initialization of the busy work, resetting the hrtimer's list node of >>>> the work and crashing kernel with null dereference within kernel/timer >>>> once work's timer is expired. It's very easy to trigger this problem by >>>> re-plugging USB cable quickly. Initialize HNP work only once to fix this >>>> trouble. >>> >>> Fully OTG compliance support has not maintained for years, what's the use case you >>> still want to use? >> >> I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it >> was crashing kernel badly. The OTG works perfectly fine without the FSM. > > You could add below at your dts to disable OTG FSM: > hnp-disable > srp-disable > adp-disable > > Since there are no users for OTG FSM, it hasn't maintained for years, > I am not sure if it still works OK. If I remember correctly, the VBUS > will be off if you enable HNP, and the device at the host port will be > disconnected, that's may not your expectation. > Since OTG FSM is known to be in a bad shape, could you please make a patch to remove it? I hope it's not enabled by default in a distro kernels.. oh no, CONFIG_USB_OTG_FSM=y at least in ArchLinux [1]. [1] https://archlinuxarm.org/packages/armv7h/linux-armv7/files/config I think we should fix that hrtimer bug, beackport the fix into stable kernels and then remove OTG FSM. Does this sound good to you?
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index 3740cf95560e..0697fde51d00 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -193,7 +193,11 @@ static void otg_start_hnp_polling(struct otg_fsm *fsm) if (!fsm->host_req_flag) return; - INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work); + if (!fsm->hnp_work_inited) { + INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work); + fsm->hnp_work_inited = true; + } + schedule_delayed_work(&fsm->hnp_polling_work, msecs_to_jiffies(T_HOST_REQ_POLL)); } diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index 3aee78dda16d..784659d4dc99 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -196,6 +196,7 @@ struct otg_fsm { struct mutex lock; u8 *host_req_flag; struct delayed_work hnp_polling_work; + bool hnp_work_inited; bool state_changed; };
The HNP work can be re-scheduled while it's still in-fly. This results in re-initialization of the busy work, resetting the hrtimer's list node of the work and crashing kernel with null dereference within kernel/timer once work's timer is expired. It's very easy to trigger this problem by re-plugging USB cable quickly. Initialize HNP work only once to fix this trouble. Cc: stable@vger.kernel.org Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/usb/common/usb-otg-fsm.c | 6 +++++- include/linux/usb/otg-fsm.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)