linux-kernel - Re: BUG: usb: obex in g_nokia.ko causing kernel panic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Nov 2013 17:40:42 +0100
From:	Pali Rohár <pali.rohar@...il.com>
To:	balbi@...com
Cc:	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Pavel Machek <pavel@....cz>,
	Aaro Koskinen <aaro.koskinen@....fi>, freemangordon@....bg,
	Sebastian Reichel <sre@...g0.de>
Subject: Re: BUG: usb: obex in g_nokia.ko causing kernel panic

On Tuesday 26 November 2013 20:03:11 Felipe Balbi wrote:
> Hi,
> 
> On Tue, Nov 26, 2013 at 12:59:13PM -0600, Felipe Balbi wrote:
> > > > diff --git a/drivers/usb/gadget/f_obex.c
> > > > b/drivers/usb/gadget/f_obex.c index ad39f1d..4af2f06
> > > > 100644 --- a/drivers/usb/gadget/f_obex.c
> > > > +++ b/drivers/usb/gadget/f_obex.c
> > > > @@ -267,10 +267,12 @@ static void obex_connect(struct
> > > > gserial *g) if (!obex->can_activate)
> > > > 
> > > >  		return;
> > > > 
> > > > +#if 0
> > > > 
> > > >  	status = usb_function_activate(&g->func);
> > > >  	if (status)
> > > >  	
> > > >  		DBG(cdev, "obex ttyGS%d function activate --> %d\n",
> > > >  		
> > > >  			obex->port_num, status);
> > > > 
> > > > +#endif
> > > > 
> > > >  }
> > > >  
> > > >  static void obex_disconnect(struct gserial *g)
> > > > 
> > > > @@ -282,10 +284,12 @@ static void obex_disconnect(struct
> > > > gserial *g) if (!obex->can_activate)
> > > > 
> > > >  		return;
> > > > 
> > > > +#if 0
> > > > 
> > > >  	status = usb_function_deactivate(&g->func);
> > > >  	if (status)
> > > >  	
> > > >  		DBG(cdev, "obex ttyGS%d function deactivate -->
> > > >  		%d\n",
> > > >  		
> > > >  			obex->port_num, status);
> > > > 
> > > > +#endif
> > > > 
> > > >  }
> > > >  
> > > >  /*-----------------------------------------------------
> > > >  ------
> > > > 
> > > > --------------*/ @@ -372,6 +376,7 @@ static int
> > > > obex_bind(struct usb_configuration *c, struct
> > > > usb_function *f) if (status)
> > > > 
> > > >  		goto fail;
> > > > 
> > > > +#if 0
> > > > 
> > > >  	/* Avoid letting this gadget enumerate until the
> > > >  	userspace
> > > >  	
> > > >  	 * OBEX server is active.
> > > >  	 */
> > > > 
> > > > @@ -381,6 +386,7 @@ static int obex_bind(struct
> > > > usb_configuration *c, struct usb_function *f)
> > > > obex->port_num, status);
> > > > 
> > > >  	else
> > > >  	
> > > >  		obex->can_activate = true;
> > > > 
> > > > +#endif
> > > > 
> > > >  	DBG(cdev, "obex ttyGS%d: %s speed IN/%s OUT/%s\n",
> > > 
> > > Hi, with above patch g_nokia.ko working and not crashing.
> > 
> > yeah, makes sense. We shouldn't call phy operations in
> > atomic context. I'll see how easy it would be to fix that.
> > Real patch coming soon(-ish).
> 
> I think we could send diff below for the -rc cycle and figure
> out a better to handle this for the merge window. Can you
> check if it also works with patch below ?
> 
> diff --git a/drivers/usb/musb/omap2430.c
> b/drivers/usb/musb/omap2430.c index 2a408cd..8aa59a2 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -659,7 +659,6 @@ static int omap2430_runtime_suspend(struct
> device *dev) OTG_INTERFSEL);
> 
>  		omap2430_low_level_exit(musb);
> -		phy_power_off(musb->phy);
>  	}
> 
>  	return 0;
> @@ -674,7 +673,6 @@ static int omap2430_runtime_resume(struct
> device *dev) omap2430_low_level_init(musb);
>  		musb_writel(musb->mregs, OTG_INTERFSEL,
>  				musb->context.otg_interfsel);
> -		phy_power_on(musb->phy);
>  	}
> 
>  	return 0;

Hi, I applied similar patch (commented usb_phy_set_suspend 
instead phy_power_on/off) on 3.12-rc5 and it working too, no 
crash. I do not have rebased n900 patches on top of 3.13 tree, so 
I cannot check it. But I think phy_power_on/off doing same as 
usb_phy_set_suspend in 3.12.

-- 
Pali Rohár
pali.rohar@...il.com

Download attachment "signature.asc " of type "application/pgp-signature" (199 bytes)