Re: [PATCH] Revert "usb: gadget: f_hid: use alloc_ep_req()"

From: Felipe Balbi <balbi@kernel.org>
To: David Lechner <david@lechnology.com>
Cc: David Lechner <david@lechnology.com>,
	"Felipe F . Tonello" <eu@felipetonello.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "usb: gadget: f_hid: use alloc_ep_req()"
Date: Tue, 03 Jan 2017 14:34:16 +0200	[thread overview]
Message-ID: <87zij8xqyf.fsf@linux.intel.com> (raw)
In-Reply-To: <1483395439-996-1-git-send-email-david@lechnology.com>

[-- Attachment #1: Type: text/plain, Size: 7060 bytes --]

Hi,

David Lechner <david@lechnology.com> writes:
> This reverts commit ba1582f22231821c57534e87b077d84adbc15dbd.
>
> I am getting a null pointer dereference when setting up an hid gadget using
> configfs. Reverting this commit fixes the crash.
>
> dmesg:
>
> [  382.406622] Unable to handle kernel NULL pointer dereference at virtual address 00000002
> [  382.406672] pgd = c3b0c000
> [  382.406695] [00000002] *pgd=c2d7e831, *pte=00000000, *ppte=00000000
> [  382.406772] Internal error: Oops: 17 [#1] PREEMPT ARM
> [  382.406793] Modules linked in: usb_f_hid usb_f_ecm usb_f_rndis u_ether d_pwm d_analog d_uart d_iic rtl8150 suart_emu snd_legoev3 snd_pcm snd_timer snd soundcore lms2012_compat legoev3_bluetooth legoev3_i2c fuse uinput libcomposite configfs
> [  382.407059] CPU: 0 PID: 485 Comm: usb-hid-gadget. Not tainted 4.9.0-ev3dev-bpo-stretch-r2-ev3-lms2012 #1
> [  382.407076] Hardware name: LEGO MINDSTORMS EV3
> [  382.407099] task: c36f7660 task.stack: c2e6c000
> [  382.407450] PC is at alloc_ep_req+0x28/0x8c [libcomposite]
> [  382.407522] LR is at kmem_cache_alloc+0x148/0x154
> [  382.407557] pc : [<bf0138d8>]    lr : [<c00c9c94>]    psr: a0000013
> sp : c2e6dd60  ip : 00000000  fp : c2e6dd7c
> [  382.407578] r10: c3bd527c  r9 : c3bd52d4  r8 : c2d132a8
> [  382.407601] r7 : bf10769c  r6 : c39a4410  r5 : 00000400  r4 : c3b3c2a0
> [  382.407623] r3 : 00000000  r2 : 00000000  r1 : ffffffe0  r0 : c3b3c2a0
> [  382.407648] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> [  382.407671] Control: 0005317f  Table: c3b0c000  DAC: 00000051
> [  382.407694] Process usb-hid-gadget. (pid: 485, stack limit = 0xc2e6c190)
> [  382.407716] Stack: (0xc2e6dd60 to 0xc2e6e000)
> [  382.407769] dd60: c2ec7654 c3bd527c 00000000 c3bd5200 c2e6ddbc c2e6dd80 bf106894 bf0138c0
> [  382.407820] dd80: c2e6de34 c2e6dd90 c000e080 c0009010 c08f0f98 c2d1331c c3bd527c c2d132a8
> [  382.407870] dda0: c2d132a8 c2d13200 c2d1331c c3bd527c c2e6dddc c2e6ddc0 bf00f844 bf106804
> [  382.407920] ddc0: c2ec7400 c3bd52d4 c2ec7654 c2d132c4 c2e6de34 c2e6dde0 bf0133a0 bf00f7c8
> [  382.407969] dde0: c2ec7400 00000000 c39a5140 c2ec768c c2e6de1c c2d1331c c3b16264 c2e6997c
> [  382.408019] de00: c3bd52d4 c2d132c8 c35ec390 c3a91400 c2ec75e0 c2ec75e0 00000000 c2ec7590
> [  382.408067] de20: 00000000 00000000 c2e6de54 c2e6de38 c0344e7c bf013134 00000000 c3a91400
> [  382.408117] de40: c2ec75e0 c37c0c00 c2e6de7c c2e6de58 c0345028 c0344e58 c37c0c00 c00a1994
> [  382.408168] de60: c2ec7400 00000011 c3ba9000 c37c0c00 c2e6dea4 c2e6de80 bf01234c c0344f18
> [  382.408216] de80: 00000011 c08f0cc0 c3ba9000 c2e6df80 00000051 c08f0cd8 c2e6dedc c2e6dea8
> [  382.408267] dea0: bf000cd0 bf0122d4 c2e6defc c1d06a00 c00109c0 c1d06a00 c2e6df80 bf004a40
> [  382.408316] dec0: 00000011 c2e6df80 c2e6c000 00000000 c2e6df4c c2e6dee0 c00d411c bf000bc0
> [  382.408366] dee0: c06999f0 c2e6dfb0 000da2b8 b6e7a000 c2e6dfac c2e6df00 c000930c c00107e0
> [  382.408415] df00: c00f45b4 c00d1aa0 c3b603c0 00000000 c3b603c0 0000000a c1d06a00 c2ff60e0
> [  382.408463] df20: c00f4f70 00000001 c1d06a00 c1d06a00 00000000 00000011 000fc408 c2e6df80
> [  382.408513] df40: c2e6df7c c2e6df50 c00d5370 c00d40fc c2e6df7c c2e6df60 c1d06a00 c1d06a00
> [  382.408562] df60: 00000011 000fc408 c000a464 00000000 c2e6dfa4 c2e6df80 c00d55cc c00d52bc
> [  382.408608] df80: 00000000 00000000 00000011 000fc408 b6e7ab40 00000004 00000000 c2e6dfa8
> [  382.408655] dfa0: c000a2c0 c00d5594 00000011 000fc408 00000001 000fc408 00000011 00000000
> [  382.408701] dfc0: 00000011 000fc408 b6e7ab40 00000004 00000011 000fc408 00000011 00000000
> [  382.408747] dfe0: 00000000 beb53734 b6da2cc0 b6dfbefc 60000010 00000001 00000000 00000000
> [  382.408756] Backtrace: 
> [  382.409175] [<bf0138b0>] (alloc_ep_req [libcomposite]) from [<bf106894>] (hidg_bind+0xa0/0x268 [usb_f_hid])
> [  382.409225]  r6:c3bd5200 r5:00000000 r4:c3bd527c r3:c2ec7654
> [  382.409591] [<bf1067f4>] (hidg_bind [usb_f_hid]) from [<bf00f844>] (usb_add_function+0x8c/0x13c [libcomposite])
> [  382.409652]  r10:c3bd527c r8:c2d1331c r7:c2d13200 r6:c2d132a8 r5:c2d132a8 r4:c3bd527c
> [  382.410191] [<bf00f7b8>] (usb_add_function [libcomposite]) from [<bf0133a0>] (configfs_composite_bind+0x27c/0x34c [libcomposite])
> [  382.410226]  r5:c2d132c4 r4:c2ec7654
> [  382.410549] [<bf013124>] (configfs_composite_bind [libcomposite]) from [<c0344e7c>] (udc_bind_to_driver+0x34/0xc0)
> [  382.410606]  r10:00000000 r9:00000000 r8:c2ec7590 r7:00000000 r6:c2ec75e0 r5:c2ec75e0
> [  382.410623]  r4:c3a91400
> [  382.410697] [<c0344e48>] (udc_bind_to_driver) from [<c0345028>] (usb_gadget_probe_driver+0x120/0x14c)
> [  382.410736]  r6:c37c0c00 r5:c2ec75e0 r4:c3a91400 r3:00000000
> [  382.411059] [<c0344f08>] (usb_gadget_probe_driver) from [<bf01234c>] (gadget_dev_desc_UDC_store+0x88/0xc0 [libcomposite])
> [  382.411105]  r7:c37c0c00 r6:c3ba9000 r5:00000011 r4:c2ec7400
> [  382.411584] [<bf0122c4>] (gadget_dev_desc_UDC_store [libcomposite]) from [<bf000cd0>] (configfs_write_file+0x120/0x154 [configfs])
> [  382.411644]  r10:c08f0cd8 r8:00000051 r7:c2e6df80 r6:c3ba9000 r5:c08f0cc0 r4:00000011
> [  382.411865] [<bf000bb0>] (configfs_write_file [configfs]) from [<c00d411c>] (__vfs_write+0x30/0x10c)
> [  382.411922]  r10:00000000 r9:c2e6c000 r8:c2e6df80 r7:00000011 r6:bf004a40 r5:c2e6df80
> [  382.411940]  r4:c1d06a00
> [  382.412001] [<c00d40ec>] (__vfs_write) from [<c00d5370>] (vfs_write+0xc4/0x150)
> [  382.412045]  r8:c2e6df80 r7:000fc408 r6:00000011 r5:00000000 r4:c1d06a00
> [  382.412103] [<c00d52ac>] (vfs_write) from [<c00d55cc>] (SyS_write+0x48/0x84)
> [  382.412153]  r10:00000000 r8:c000a464 r7:000fc408 r6:00000011 r5:c1d06a00 r4:c1d06a00
> [  382.412213] [<c00d5584>] (SyS_write) from [<c000a2c0>] (ret_fast_syscall+0x0/0x38)
> [  382.412250]  r7:00000004 r6:b6e7ab40 r5:000fc408 r4:00000011
> [  382.412293] Code: eb4cc3d0 e2504000 0a000016 e5963024 (e1d320d2) 
> [  382.437688] ---[ end trace 3671b14cbf5571de ]---
>
> ---
>
>  drivers/usb/gadget/function/f_hid.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index e2966f8..aa1c199 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -617,10 +617,14 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
>  
>  	/* preallocate request and buffer */
>  	status = -ENOMEM;
> -	hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);
> +	hidg->req = usb_ep_alloc_request(hidg->in_ep, GFP_KERNEL);
>  	if (!hidg->req)
>  		goto fail;
>  
> +	hidg->req->buf = kmalloc(hidg->report_length, GFP_KERNEL);
> +	if (!hidg->req->buf)
> +		goto fail;
> +
>  	/* set descriptor dynamic values */
>  	hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
>  	hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;

Felipe T., any comments?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]