diff mbox series

usb: musb: Remove unnecessary check of musb->hcd in musb_handle_intr_connect()

Message ID 20191217095023.6878-1-baijiaju1990@gmail.com (mailing list archive)
State New, archived
Headers show
Series usb: musb: Remove unnecessary check of musb->hcd in musb_handle_intr_connect() | expand

Commit Message

Jia-Ju Bai Dec. 17, 2019, 9:50 a.m. UTC
In musb_handle_intr_connect(), musb->hcd should be non-null, 
so the check of musb->hcd on line 783 could be dropped.

Thank Greg for good advice.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 drivers/usb/musb/musb_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Greg KH Dec. 17, 2019, 9:59 a.m. UTC | #1
On Tue, Dec 17, 2019 at 05:50:23PM +0800, Jia-Ju Bai wrote:
> In musb_handle_intr_connect(), musb->hcd should be non-null, 
> so the check of musb->hcd on line 783 could be dropped.

Have you verified that this is always the case?  How did you do that?

thanks,

greg k-h
Jia-Ju Bai Dec. 17, 2019, 12:27 p.m. UTC | #2
On 2019/12/17 17:59, Greg KH wrote:
> On Tue, Dec 17, 2019 at 05:50:23PM +0800, Jia-Ju Bai wrote:
>> In musb_handle_intr_connect(), musb->hcd should be non-null,
>> so the check of musb->hcd on line 783 could be dropped.
> Have you verified that this is always the case?  How did you do that?

Ah, so sorry, I misunderstood your last message...
I have not verified it, but I thought you made sure of it from your last 
message, sorry...

In my opinion, adding an if check is safer, as shown in my last patch:
"usb: musb: Fix a possible null-pointer dereference in 
musb_handle_intr_connect()"

I recall that Bin Liu has applied my last patch:
https://patchwork.kernel.org/patch/11283007/

Maybe we can listen to his opinion or other people's opinions?


Best wishes,
Jia-Ju Bai
Bin Liu Dec. 17, 2019, 7:15 p.m. UTC | #3
Hi,

On Tue, Dec 17, 2019 at 08:27:09PM +0800, Jia-Ju Bai wrote:
> 
> 
> On 2019/12/17 17:59, Greg KH wrote:
> > On Tue, Dec 17, 2019 at 05:50:23PM +0800, Jia-Ju Bai wrote:
> > > In musb_handle_intr_connect(), musb->hcd should be non-null,
> > > so the check of musb->hcd on line 783 could be dropped.
> > Have you verified that this is always the case?  How did you do that?
> 
> Ah, so sorry, I misunderstood your last message...
> I have not verified it, but I thought you made sure of it from your last
> message, sorry...
> 
> In my opinion, adding an if check is safer, as shown in my last patch:
> "usb: musb: Fix a possible null-pointer dereference in
> musb_handle_intr_connect()"
> 
> I recall that Bin Liu has applied my last patch:
> https://patchwork.kernel.org/patch/11283007/
> 
> Maybe we can listen to his opinion or other people's opinions?

The code path this patch touches is in OTG_STATE_B_WAIT_ACON branch,
which is part of the OTG HNP handling, but OTG protocol support has been
disabled since commit 0a9134bd733b, so technically this code path won't
be reached.

Let's not bother with this ->hcd pointer check, and wait for OTG code
cleanup in this driver.

-Bin.
diff mbox series

Patch

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 15cca912c53e..ce57f253a8e1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -780,8 +780,7 @@  static void musb_handle_intr_connect(struct musb *musb, u8 devctl, u8 int_usb)
 		musb_dbg(musb, "HNP: CONNECT, now b_host");
 b_host:
 		musb->xceiv->otg->state = OTG_STATE_B_HOST;
-		if (musb->hcd)
-			musb->hcd->self.is_b_host = 1;
+		musb->hcd->self.is_b_host = 1;
 		del_timer(&musb->otg_timer);
 		break;
 	default: