[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Songjun Wu]

Executing the 'insmod g_hid.ko', then executing the
'rmmod g_hid.ko', the NULL pointer oops will be triggered.

When unloading the module 'g_hid.ko', the urb request will be
dequeued and the completion routine will be excuted. If no urb
packet, the urb request will not be added to the endpoint queue
and the completion routine pointer in urb request is NULL.
Accessing to the NULL function pointer will cause the oops issue.
Add the code to check the urb request is in the endpoint queue
or not.If the urb request is not in the endpoint queue, a negative
error code will be returned.

This bug was introduced since the file 'atmel_usba_udc.c' was
initialized. Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable@vger.kernel.org # always been there...

oops dump log is shown in the following.
Unable to handle kernel NULL pointer dereference at virtual
address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce88237..48629cc 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
1.7.9.5

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Songjun Wu]

Executing the 'insmod g_hid.ko', then executing the
'rmmod g_hid.ko', the NULL pointer oops will be triggered.

When unloading the module 'g_hid.ko', the urb request will be
dequeued and the completion routine will be excuted. If no urb
packet, the urb request will not be added to the endpoint queue
and the completion routine pointer in urb request is NULL.
Accessing to the NULL function pointer will cause the oops issue.
Add the code to check the urb request is in the endpoint queue
or not.If the urb request is not in the endpoint queue, a negative
error code will be returned.

This bug was introduced since the file 'atmel_usba_udc.c' was
initialized. Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable at vger.kernel.org # always been there...

oops dump log is shown in the following.
Unable to handle kernel NULL pointer dereference at virtual
address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce88237..48629cc 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
1.7.9.5

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

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

Hi,

On Tue, Dec 30, 2014 at 05:49:27PM +0800, Songjun Wu wrote:
> Executing the 'insmod g_hid.ko', then executing the
> 'rmmod g_hid.ko', the NULL pointer oops will be triggered.
> 
> When unloading the module 'g_hid.ko', the urb request will be
> dequeued and the completion routine will be excuted. If no urb
> packet, the urb request will not be added to the endpoint queue
> and the completion routine pointer in urb request is NULL.
> Accessing to the NULL function pointer will cause the oops issue.
> Add the code to check the urb request is in the endpoint queue
> or not.If the urb request is not in the endpoint queue, a negative
> error code will be returned.
> 
> This bug was introduced since the file 'atmel_usba_udc.c' was
> initialized. Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
> Cc: stable@vger.kernel.org # always been there...

this is not the way you write this. There are tons of examples in the
very git tree you're using of how to do this. Just run:

$ git log --grep "Fixes:"

Also have a read at Documentation/SubmittingPatches and search for
"Fixes" in that file. You might also want to look at
Documentation/stable-kernel-rules.txt to figure out how to properly Cc
stable.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

Hi,

On Tue, Dec 30, 2014 at 05:49:27PM +0800, Songjun Wu wrote:
> Executing the 'insmod g_hid.ko', then executing the
> 'rmmod g_hid.ko', the NULL pointer oops will be triggered.
> 
> When unloading the module 'g_hid.ko', the urb request will be
> dequeued and the completion routine will be excuted. If no urb
> packet, the urb request will not be added to the endpoint queue
> and the completion routine pointer in urb request is NULL.
> Accessing to the NULL function pointer will cause the oops issue.
> Add the code to check the urb request is in the endpoint queue
> or not.If the urb request is not in the endpoint queue, a negative
> error code will be returned.
> 
> This bug was introduced since the file 'atmel_usba_udc.c' was
> initialized. Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
> Cc: stable at vger.kernel.org # always been there...

this is not the way you write this. There are tons of examples in the
very git tree you're using of how to do this. Just run:

$ git log --grep "Fixes:"

Also have a read at Documentation/SubmittingPatches and search for
"Fixes" in that file. You might also want to look at
Documentation/stable-kernel-rules.txt to figure out how to properly Cc
stable.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/f33afffd/attachment.sig>

[RESEND PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Nicolas Ferre]

From: Songjun Wu <songjun.wu@atmel.com>

When unloading the module 'g_hid.ko', the urb request will be dequeued and the
completion routine will be excuted. If there is no urb packet, the urb request
will not be added to the endpoint queue and the completion routine pointer in
urb request is NULL.
Accessing to this NULL function pointer will cause the Oops issue reported
below.
Add the code to check if the urb request is in the endpoint queue
or not. If the urb request is not in the endpoint queue, a negative
error code will be returned.

Here is the Oops log:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 bf0031d0
[..]
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] (request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] (usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c [usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] (remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] (__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] (usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] (usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] (hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] (SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
Code: bad PC value

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
[nicolas.ferre@atmel.com: reworked the commit message]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Fixes: 914a3f3b3754 ("USB: add atmel_usba_udc driver")
Cc: <stable@vger.kernel.org> # 2.6.x-ish
---
Felipe,

I tried to collect all the information needed. Please tell me if it's what you
expect from us.

Songjun,

I took a summary of your previous emails + the Oops log without breaking lines.
The "Fixes" tag goes into the last part of the email. Thanks a lot for your
fix.

Bye,


 drivers/usb/gadget/udc/atmel_usba_udc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce882371786b..48629cc3d6f8 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
2.1.3

[RESEND PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Nicolas Ferre]

From: Songjun Wu <songjun.wu@atmel.com>

When unloading the module 'g_hid.ko', the urb request will be dequeued and the
completion routine will be excuted. If there is no urb packet, the urb request
will not be added to the endpoint queue and the completion routine pointer in
urb request is NULL.
Accessing to this NULL function pointer will cause the Oops issue reported
below.
Add the code to check if the urb request is in the endpoint queue
or not. If the urb request is not in the endpoint queue, a negative
error code will be returned.

Here is the Oops log:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 bf0031d0
[..]
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] (request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] (usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c [usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] (remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] (__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] (usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] (usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] (hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] (SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
Code: bad PC value

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
[nicolas.ferre at atmel.com: reworked the commit message]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Fixes: 914a3f3b3754 ("USB: add atmel_usba_udc driver")
Cc: <stable@vger.kernel.org> # 2.6.x-ish
---
Felipe,

I tried to collect all the information needed. Please tell me if it's what you
expect from us.

Songjun,

I took a summary of your previous emails + the Oops log without breaking lines.
The "Fixes" tag goes into the last part of the email. Thanks a lot for your
fix.

Bye,


 drivers/usb/gadget/udc/atmel_usba_udc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce882371786b..48629cc3d6f8 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
2.1.3

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

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

Hi,

On Mon, Dec 29, 2014 at 05:42:01PM +0800, Wu, Songjun wrote:
> 
> 
> On 12/26/2014 23:27, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
> >>
> >>在 12/24/2014 00:24, Felipe Balbi 写道:
> >>>On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> >>>>When unloading the module, the urb request will be dequeued
> >>>>and the completion routine will be excuted.
> >>>>If no urb packet, the urb request will not be added to the endpoint queue
> >>>>and the completion routine pointer in urb request is NULL.
> >>>>Accessing to the NULL function pointer will cause the oops issue.
> >>>>Add the code to check the urb request is in the endpoint queue or not.
> >>>>If the urb request is not in the endpoint queue, a negative error code
> >>>>will be returned.
> >>>
> >>>have you triggered the NULL pointer oops ? Care to add it to the commit
> >>>log.
> >>
> >>Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
> >>NULL pointer oops will be triggered.
> >
> >what about all my other queries below and what about adding the oops
> >dump to commit log ?
> >
> >>>Also, which commit is this fixing ? Does this need to be backported ?
> >>>When was the bug introduced ?
> >
> Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
> Cc: stable@vger.kernel.org # always been there...
> This bug was introduced since the file 'atmel_usba_udc.c' was initialized.
> 
> The oops dump log is shown in the following.
> # insmod g_hid.ko
> g_hid gadget: HID Gadget, version: 2010/03/16
> g_hid gadget: g_hid ready
> # rmmod g_hid.ko
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = dedf0000
> [00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 80000007 [#1] ARM
> Modules linked in: g_hid(-) usb_f_hid libcomposite
> CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
> Hardware name: Atmel SAMA5 (Device Tree)
> task: df6b1100 ti: dedf6000 task.ti: dedf6000
> PC is at 0x0
> LR is at usb_gadget_giveback_request+0xc/0x10
> pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
> sp : dedf7eb0  ip : df572634  fp : 00000000
> r10: 00000000  r9 : df52e210  r8 : 60000013
> r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
> r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
> Process rmmod (pid: 923, stack limit = 0xdedf6230)
> Stack: (0xdedf7eb0 to 0xdedf8000)
> 7ea0:                                     00000000 c02adbbc df572580
> deced608
> 7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000
> df6be27c
> 7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000
> bf010c00
> 7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8
> bf0031d0
> 7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000
> c02ad570
> 7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022
> 69685f67
> 7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c
> c008370c
> 7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588
> 69685f67
> 7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78
> 00000880
> 7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f
> 00000000
> 7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821
> 3fffdc21
> [<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>]
> (request_complete+0x64/0x88)
> [<c02adbbc>] (request_complete) from [<c02aed3c>]
> (usba_ep_dequeue+0x70/0x128)
> [<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c
> [usb_f_hid])
> [<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>]
> (remove_config.isra.6+0x98/0x9c [libcomposite])
> [<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>]
> (__composite_unbind+0x34/0x98 [libcomposite])
> [<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>]
> (usb_gadget_remove_driver+0x50/0x78)
> [<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>]
> (usb_gadget_unregister_driver+0x64/0x94)
> [<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>]
> (hidg_cleanup+0x10/0x34 [g_hid])
> [<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>]
> (SyS_delete_module+0x118/0x19c)
> [<c0056748>] (SyS_delete_module) from [<c000e3c0>]
> (ret_fast_syscall+0x0/0x30)
> Code: bad PC value
> ---[ end trace dd1fcf365005ba79 ]---
> Segmentation fault

you need to resend the patch with all of this properly placed on your
commit log.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

Hi,

On Mon, Dec 29, 2014 at 05:42:01PM +0800, Wu, Songjun wrote:
> 
> 
> On 12/26/2014 23:27, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
> >>
> >>? 12/24/2014 00:24, Felipe Balbi ??:
> >>>On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> >>>>When unloading the module, the urb request will be dequeued
> >>>>and the completion routine will be excuted.
> >>>>If no urb packet, the urb request will not be added to the endpoint queue
> >>>>and the completion routine pointer in urb request is NULL.
> >>>>Accessing to the NULL function pointer will cause the oops issue.
> >>>>Add the code to check the urb request is in the endpoint queue or not.
> >>>>If the urb request is not in the endpoint queue, a negative error code
> >>>>will be returned.
> >>>
> >>>have you triggered the NULL pointer oops ? Care to add it to the commit
> >>>log.
> >>
> >>Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
> >>NULL pointer oops will be triggered.
> >
> >what about all my other queries below and what about adding the oops
> >dump to commit log ?
> >
> >>>Also, which commit is this fixing ? Does this need to be backported ?
> >>>When was the bug introduced ?
> >
> Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
> Cc: stable at vger.kernel.org # always been there...
> This bug was introduced since the file 'atmel_usba_udc.c' was initialized.
> 
> The oops dump log is shown in the following.
> # insmod g_hid.ko
> g_hid gadget: HID Gadget, version: 2010/03/16
> g_hid gadget: g_hid ready
> # rmmod g_hid.ko
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = dedf0000
> [00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 80000007 [#1] ARM
> Modules linked in: g_hid(-) usb_f_hid libcomposite
> CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
> Hardware name: Atmel SAMA5 (Device Tree)
> task: df6b1100 ti: dedf6000 task.ti: dedf6000
> PC is at 0x0
> LR is at usb_gadget_giveback_request+0xc/0x10
> pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
> sp : dedf7eb0  ip : df572634  fp : 00000000
> r10: 00000000  r9 : df52e210  r8 : 60000013
> r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
> r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
> Process rmmod (pid: 923, stack limit = 0xdedf6230)
> Stack: (0xdedf7eb0 to 0xdedf8000)
> 7ea0:                                     00000000 c02adbbc df572580
> deced608
> 7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000
> df6be27c
> 7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000
> bf010c00
> 7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8
> bf0031d0
> 7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000
> c02ad570
> 7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022
> 69685f67
> 7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c
> c008370c
> 7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588
> 69685f67
> 7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78
> 00000880
> 7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f
> 00000000
> 7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821
> 3fffdc21
> [<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>]
> (request_complete+0x64/0x88)
> [<c02adbbc>] (request_complete) from [<c02aed3c>]
> (usba_ep_dequeue+0x70/0x128)
> [<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c
> [usb_f_hid])
> [<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>]
> (remove_config.isra.6+0x98/0x9c [libcomposite])
> [<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>]
> (__composite_unbind+0x34/0x98 [libcomposite])
> [<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>]
> (usb_gadget_remove_driver+0x50/0x78)
> [<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>]
> (usb_gadget_unregister_driver+0x64/0x94)
> [<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>]
> (hidg_cleanup+0x10/0x34 [g_hid])
> [<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>]
> (SyS_delete_module+0x118/0x19c)
> [<c0056748>] (SyS_delete_module) from [<c000e3c0>]
> (ret_fast_syscall+0x0/0x30)
> Code: bad PC value
> ---[ end trace dd1fcf365005ba79 ]---
> Segmentation fault

you need to resend the patch with all of this properly placed on your
commit log.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141229/3369ef15/attachment.sig>

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

On 12/26/2014 23:27, Felipe Balbi wrote:
> Hi,
>
> On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
>>
>> 在 12/24/2014 00:24, Felipe Balbi 写道:
>>> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>>>> When unloading the module, the urb request will be dequeued
>>>> and the completion routine will be excuted.
>>>> If no urb packet, the urb request will not be added to the endpoint queue
>>>> and the completion routine pointer in urb request is NULL.
>>>> Accessing to the NULL function pointer will cause the oops issue.
>>>> Add the code to check the urb request is in the endpoint queue or not.
>>>> If the urb request is not in the endpoint queue, a negative error code
>>>> will be returned.
>>>
>>> have you triggered the NULL pointer oops ? Care to add it to the commit
>>> log.
>>
>> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
>> NULL pointer oops will be triggered.
>
> what about all my other queries below and what about adding the oops
> dump to commit log ?
>
>>> Also, which commit is this fixing ? Does this need to be backported ?
>>> When was the bug introduced ?
>
Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable@vger.kernel.org # always been there...
This bug was introduced since the file 'atmel_usba_udc.c' was initialized.

The oops dump log is shown in the following.
# insmod g_hid.ko
g_hid gadget: HID Gadget, version: 2010/03/16
g_hid gadget: g_hid ready
# rmmod g_hid.ko
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 
deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 
df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 
bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 
bf0031d0
7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000 
c02ad570
7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022 
69685f67
7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c 
c008370c
7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588 
69685f67
7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78 
00000880
7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f 
00000000
7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821 
3fffdc21
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] 
(request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] 
(usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c 
[usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] 
(remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] 
(__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] 
(usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] 
(usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] 
(hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] 
(SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] 
(ret_fast_syscall+0x0/0x30)
Code: bad PC value
---[ end trace dd1fcf365005ba79 ]---
Segmentation fault
#

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

On 12/26/2014 23:27, Felipe Balbi wrote:
> Hi,
>
> On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
>>
>> ? 12/24/2014 00:24, Felipe Balbi ??:
>>> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>>>> When unloading the module, the urb request will be dequeued
>>>> and the completion routine will be excuted.
>>>> If no urb packet, the urb request will not be added to the endpoint queue
>>>> and the completion routine pointer in urb request is NULL.
>>>> Accessing to the NULL function pointer will cause the oops issue.
>>>> Add the code to check the urb request is in the endpoint queue or not.
>>>> If the urb request is not in the endpoint queue, a negative error code
>>>> will be returned.
>>>
>>> have you triggered the NULL pointer oops ? Care to add it to the commit
>>> log.
>>
>> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
>> NULL pointer oops will be triggered.
>
> what about all my other queries below and what about adding the oops
> dump to commit log ?
>
>>> Also, which commit is this fixing ? Does this need to be backported ?
>>> When was the bug introduced ?
>
Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable at vger.kernel.org # always been there...
This bug was introduced since the file 'atmel_usba_udc.c' was initialized.

The oops dump log is shown in the following.
# insmod g_hid.ko
g_hid gadget: HID Gadget, version: 2010/03/16
g_hid gadget: g_hid ready
# rmmod g_hid.ko
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 
deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 
df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 
bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 
bf0031d0
7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000 
c02ad570
7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022 
69685f67
7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c 
c008370c
7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588 
69685f67
7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78 
00000880
7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f 
00000000
7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821 
3fffdc21
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] 
(request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] 
(usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c 
[usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] 
(remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] 
(__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] 
(usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] 
(usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] 
(hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] 
(SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] 
(ret_fast_syscall+0x0/0x30)
Code: bad PC value
---[ end trace dd1fcf365005ba79 ]---
Segmentation fault
#

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

On 12/26/2014 23:27, Felipe Balbi wrote:
> Hi,
>
> On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
>>
>> 在 12/24/2014 00:24, Felipe Balbi 写道:
>>> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>>>> When unloading the module, the urb request will be dequeued
>>>> and the completion routine will be excuted.
>>>> If no urb packet, the urb request will not be added to the endpoint queue
>>>> and the completion routine pointer in urb request is NULL.
>>>> Accessing to the NULL function pointer will cause the oops issue.
>>>> Add the code to check the urb request is in the endpoint queue or not.
>>>> If the urb request is not in the endpoint queue, a negative error code
>>>> will be returned.
>>>
>>> have you triggered the NULL pointer oops ? Care to add it to the commit
>>> log.
>>
>> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
>> NULL pointer oops will be triggered.
>
> what about all my other queries below and what about adding the oops
> dump to commit log ?
>
>>> Also, which commit is this fixing ? Does this need to be backported ?
>>> When was the bug introduced ?
>
Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable@vger.kernel.org # always been there...

This bug was introduced since the file atmel_usba_udc.c was initialized.

The oops dump log is shown in the following.
# insmod libcomposite.ko
d usb_f_hid.ko
insmod g_hid.ko
# insmod usb_f_hid.ko
# insmod g_hid.ko
g_hid gadget: HID Gadget, version: 2010/03/16
g_hid gadget: g_hid ready
# rmmod g_hid.ko
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 
deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 
df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 
bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 
bf0031d0
7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000 
c02ad570
7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022 
69685f67
7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c 
c008370c
7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588 
69685f67
7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78 
00000880
7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f 
00000000
7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821 
3fffdc21
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] 
(request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] 
(usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c 
[usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] 
(remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] 
(__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] 
(usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] 
(usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] 
(hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] 
(SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] 
(ret_fast_syscall+0x0/0x30)
Code: bad PC value
---[ end trace dd1fcf365005ba79 ]---
Segmentation fault
#

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

On 12/26/2014 23:27, Felipe Balbi wrote:
> Hi,
>
> On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
>>
>> ? 12/24/2014 00:24, Felipe Balbi ??:
>>> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>>>> When unloading the module, the urb request will be dequeued
>>>> and the completion routine will be excuted.
>>>> If no urb packet, the urb request will not be added to the endpoint queue
>>>> and the completion routine pointer in urb request is NULL.
>>>> Accessing to the NULL function pointer will cause the oops issue.
>>>> Add the code to check the urb request is in the endpoint queue or not.
>>>> If the urb request is not in the endpoint queue, a negative error code
>>>> will be returned.
>>>
>>> have you triggered the NULL pointer oops ? Care to add it to the commit
>>> log.
>>
>> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
>> NULL pointer oops will be triggered.
>
> what about all my other queries below and what about adding the oops
> dump to commit log ?
>
>>> Also, which commit is this fixing ? Does this need to be backported ?
>>> When was the bug introduced ?
>
Fixes: 914a3f3b3754 (USB: add atmel_usba_udc driver)
Cc: stable at vger.kernel.org # always been there...

This bug was introduced since the file atmel_usba_udc.c was initialized.

The oops dump log is shown in the following.
# insmod libcomposite.ko
d usb_f_hid.ko
insmod g_hid.ko
# insmod usb_f_hid.ko
# insmod g_hid.ko
g_hid gadget: HID Gadget, version: 2010/03/16
g_hid gadget: g_hid ready
# rmmod g_hid.ko
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = dedf0000
[00000000] *pgd=3ede5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in: g_hid(-) usb_f_hid libcomposite
CPU: 0 PID: 923 Comm: rmmod Not tainted 3.18.0+ #2
Hardware name: Atmel SAMA5 (Device Tree)
task: df6b1100 ti: dedf6000 task.ti: dedf6000
PC is at 0x0
LR is at usb_gadget_giveback_request+0xc/0x10
pc : [<00000000>]    lr : [<c02ace88>]    psr: 60000093
sp : dedf7eb0  ip : df572634  fp : 00000000
r10: 00000000  r9 : df52e210  r8 : 60000013
r7 : df6a9858  r6 : df52e210  r5 : df6a9858  r4 : df572600
r3 : 00000000  r2 : ffffff98  r1 : df572600  r0 : df6a9868
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: 3edf0059  DAC: 00000015
Process rmmod (pid: 923, stack limit = 0xdedf6230)
Stack: (0xdedf7eb0 to 0xdedf8000)
7ea0:                                     00000000 c02adbbc df572580 
deced608
7ec0: df572600 df6a9868 df572634 c02aed3c df577c00 c01b8608 00000000 
df6be27c
7ee0: 00200200 00100100 bf0162f4 c000e544 dedf6000 00000000 00000000 
bf010c00
7f00: bf0162cc bf00159c 00000000 df572980 df52e218 00000001 df5729b8 
bf0031d0
7f20: bf003234 df52e400 df52e400 a0000013 00000081 c02acee0 00000000 
c02ad570
7f40: bf0160b0 bf016290 00000000 bf0160c0 bf0167a0 c0056748 00000022 
69685f67
7f60: b6ff0064 00000001 b6ff3000 00000000 dedf6000 00000000 bebaaa4c 
c008370c
7f80: 00100871 c0081078 00000022 df5f31e8 b6ff4518 00000000 0001c588 
69685f67
7fa0: b6ff0064 c000e3c0 0001c588 69685f67 bebaab78 00000880 bebaab78 
00000880
7fc0: 0001c588 69685f67 b6ff0064 00000081 bebaae14 00000000 0000009f 
00000000
7fe0: bebaab70 bebaab60 0001c464 b6f69ba0 60000010 bebaab78 3fffd821 
3fffdc21
[<c02ace88>] (usb_gadget_giveback_request) from [<c02adbbc>] 
(request_complete+0x64/0x88)
[<c02adbbc>] (request_complete) from [<c02aed3c>] 
(usba_ep_dequeue+0x70/0x128)
[<c02aed3c>] (usba_ep_dequeue) from [<bf010c00>] (hidg_unbind+0x50/0x7c 
[usb_f_hid])
[<bf010c00>] (hidg_unbind [usb_f_hid]) from [<bf00159c>] 
(remove_config.isra.6+0x98/0x9c [libcomposite])
[<bf00159c>] (remove_config.isra.6 [libcomposite]) from [<bf0031d0>] 
(__composite_unbind+0x34/0x98 [libcomposite])
[<bf0031d0>] (__composite_unbind [libcomposite]) from [<c02acee0>] 
(usb_gadget_remove_driver+0x50/0x78)
[<c02acee0>] (usb_gadget_remove_driver) from [<c02ad570>] 
(usb_gadget_unregister_driver+0x64/0x94)
[<c02ad570>] (usb_gadget_unregister_driver) from [<bf0160c0>] 
(hidg_cleanup+0x10/0x34 [g_hid])
[<bf0160c0>] (hidg_cleanup [g_hid]) from [<c0056748>] 
(SyS_delete_module+0x118/0x19c)
[<c0056748>] (SyS_delete_module) from [<c000e3c0>] 
(ret_fast_syscall+0x0/0x30)
Code: bad PC value
---[ end trace dd1fcf365005ba79 ]---
Segmentation fault
#

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

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

Hi,

On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
> 
> 在 12/24/2014 00:24, Felipe Balbi 写道:
> >On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> >>When unloading the module, the urb request will be dequeued
> >>and the completion routine will be excuted.
> >>If no urb packet, the urb request will not be added to the endpoint queue
> >>and the completion routine pointer in urb request is NULL.
> >>Accessing to the NULL function pointer will cause the oops issue.
> >>Add the code to check the urb request is in the endpoint queue or not.
> >>If the urb request is not in the endpoint queue, a negative error code
> >>will be returned.
> >
> >have you triggered the NULL pointer oops ? Care to add it to the commit
> >log.
> 
> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
> NULL pointer oops will be triggered.

what about all my other queries below and what about adding the oops
dump to commit log ?

> >Also, which commit is this fixing ? Does this need to be backported ?
> >When was the bug introduced ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

Hi,

On Wed, Dec 24, 2014 at 09:14:53AM +0800, Wu, Songjun wrote:
> 
> ? 12/24/2014 00:24, Felipe Balbi ??:
> >On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> >>When unloading the module, the urb request will be dequeued
> >>and the completion routine will be excuted.
> >>If no urb packet, the urb request will not be added to the endpoint queue
> >>and the completion routine pointer in urb request is NULL.
> >>Accessing to the NULL function pointer will cause the oops issue.
> >>Add the code to check the urb request is in the endpoint queue or not.
> >>If the urb request is not in the endpoint queue, a negative error code
> >>will be returned.
> >
> >have you triggered the NULL pointer oops ? Care to add it to the commit
> >log.
> 
> Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', the
> NULL pointer oops will be triggered.

what about all my other queries below and what about adding the oops
dump to commit log ?

> >Also, which commit is this fixing ? Does this need to be backported ?
> >When was the bug introduced ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141226/b3c6d350/attachment.sig>

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

在 12/24/2014 00:24, Felipe Balbi 写道:
> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>> When unloading the module, the urb request will be dequeued
>> and the completion routine will be excuted.
>> If no urb packet, the urb request will not be added to the endpoint queue
>> and the completion routine pointer in urb request is NULL.
>> Accessing to the NULL function pointer will cause the oops issue.
>> Add the code to check the urb request is in the endpoint queue or not.
>> If the urb request is not in the endpoint queue, a negative error code
>> will be returned.
>
> have you triggered the NULL pointer oops ? Care to add it to the commit
> log.

Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', 
the NULL pointer oops will be triggered.

>
> Also, which commit is this fixing ? Does this need to be backported ?
> When was the bug introduced ?
>
>> Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
>> ---
>>   drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> index ce88237..48629cc 100644
>> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> @@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>>   {
>>   	struct usba_ep *ep = to_usba_ep(_ep);
>>   	struct usba_udc *udc = ep->udc;
>> -	struct usba_request *req = to_usba_req(_req);
>> +	struct usba_request *req;
>>   	unsigned long flags;
>>   	u32 status;
>>
>> @@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>>
>>   	spin_lock_irqsave(&udc->lock, flags);
>>
>> +	list_for_each_entry(req, &ep->queue, queue) {
>> +		if (&req->req == _req)
>> +			break;
>> +	}
>> +
>> +	if (&req->req != _req) {
>> +		spin_unlock_irqrestore(&udc->lock, flags);
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (req->using_dma) {
>>   		/*
>>   		 * If this request is currently being transferred,
>> --
>> 1.7.9.5
>>
>

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Wu, Songjun]

? 12/24/2014 00:24, Felipe Balbi ??:
> On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
>> When unloading the module, the urb request will be dequeued
>> and the completion routine will be excuted.
>> If no urb packet, the urb request will not be added to the endpoint queue
>> and the completion routine pointer in urb request is NULL.
>> Accessing to the NULL function pointer will cause the oops issue.
>> Add the code to check the urb request is in the endpoint queue or not.
>> If the urb request is not in the endpoint queue, a negative error code
>> will be returned.
>
> have you triggered the NULL pointer oops ? Care to add it to the commit
> log.

Executing the 'insmod g_hid.ko', then executing the 'rmmod g_hid.ko', 
the NULL pointer oops will be triggered.

>
> Also, which commit is this fixing ? Does this need to be backported ?
> When was the bug introduced ?
>
>> Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
>> ---
>>   drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> index ce88237..48629cc 100644
>> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> @@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>>   {
>>   	struct usba_ep *ep = to_usba_ep(_ep);
>>   	struct usba_udc *udc = ep->udc;
>> -	struct usba_request *req = to_usba_req(_req);
>> +	struct usba_request *req;
>>   	unsigned long flags;
>>   	u32 status;
>>
>> @@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>>
>>   	spin_lock_irqsave(&udc->lock, flags);
>>
>> +	list_for_each_entry(req, &ep->queue, queue) {
>> +		if (&req->req == _req)
>> +			break;
>> +	}
>> +
>> +	if (&req->req != _req) {
>> +		spin_unlock_irqrestore(&udc->lock, flags);
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (req->using_dma) {
>>   		/*
>>   		 * If this request is currently being transferred,
>> --
>> 1.7.9.5
>>
>

Re: [PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

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

On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> When unloading the module, the urb request will be dequeued
> and the completion routine will be excuted.
> If no urb packet, the urb request will not be added to the endpoint queue
> and the completion routine pointer in urb request is NULL.
> Accessing to the NULL function pointer will cause the oops issue.
> Add the code to check the urb request is in the endpoint queue or not.
> If the urb request is not in the endpoint queue, a negative error code
> will be returned.

have you triggered the NULL pointer oops ? Care to add it to the commit
log.

Also, which commit is this fixing ? Does this need to be backported ?
When was the bug introduced ?

> Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
> ---
>  drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index ce88237..48629cc 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  {
>  	struct usba_ep *ep = to_usba_ep(_ep);
>  	struct usba_udc *udc = ep->udc;
> -	struct usba_request *req = to_usba_req(_req);
> +	struct usba_request *req;
>  	unsigned long flags;
>  	u32 status;
>  
> @@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  
>  	spin_lock_irqsave(&udc->lock, flags);
>  
> +	list_for_each_entry(req, &ep->queue, queue) {
> +		if (&req->req == _req)
> +			break;
> +	}
> +
> +	if (&req->req != _req) {
> +		spin_unlock_irqrestore(&udc->lock, flags);
> +		return -EINVAL;
> +	}
> +
>  	if (req->using_dma) {
>  		/*
>  		 * If this request is currently being transferred,
> -- 
> 1.7.9.5
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Felipe Balbi]

On Mon, Dec 22, 2014 at 05:26:14PM +0800, Songjun Wu wrote:
> When unloading the module, the urb request will be dequeued
> and the completion routine will be excuted.
> If no urb packet, the urb request will not be added to the endpoint queue
> and the completion routine pointer in urb request is NULL.
> Accessing to the NULL function pointer will cause the oops issue.
> Add the code to check the urb request is in the endpoint queue or not.
> If the urb request is not in the endpoint queue, a negative error code
> will be returned.

have you triggered the NULL pointer oops ? Care to add it to the commit
log.

Also, which commit is this fixing ? Does this need to be backported ?
When was the bug introduced ?

> Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
> ---
>  drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index ce88237..48629cc 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  {
>  	struct usba_ep *ep = to_usba_ep(_ep);
>  	struct usba_udc *udc = ep->udc;
> -	struct usba_request *req = to_usba_req(_req);
> +	struct usba_request *req;
>  	unsigned long flags;
>  	u32 status;
>  
> @@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  
>  	spin_lock_irqsave(&udc->lock, flags);
>  
> +	list_for_each_entry(req, &ep->queue, queue) {
> +		if (&req->req == _req)
> +			break;
> +	}
> +
> +	if (&req->req != _req) {
> +		spin_unlock_irqrestore(&udc->lock, flags);
> +		return -EINVAL;
> +	}
> +
>  	if (req->using_dma) {
>  		/*
>  		 * If this request is currently being transferred,
> -- 
> 1.7.9.5
> 

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141223/bdbb74a1/attachment.sig>

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Songjun Wu]

When unloading the module, the urb request will be dequeued
and the completion routine will be excuted.
If no urb packet, the urb request will not be added to the endpoint queue
and the completion routine pointer in urb request is NULL.
Accessing to the NULL function pointer will cause the oops issue.
Add the code to check the urb request is in the endpoint queue or not.
If the urb request is not in the endpoint queue, a negative error code
will be returned.

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce88237..48629cc 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
1.7.9.5

[PATCH] USB: gadget: udc: atmel: fix possible oops when unloading module

[Songjun Wu]

When unloading the module, the urb request will be dequeued
and the completion routine will be excuted.
If no urb packet, the urb request will not be added to the endpoint queue
and the completion routine pointer in urb request is NULL.
Accessing to the NULL function pointer will cause the oops issue.
Add the code to check the urb request is in the endpoint queue or not.
If the urb request is not in the endpoint queue, a negative error code
will be returned.

Signed-off-by: Songjun Wu <songjun.wu@atmel.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce88237..48629cc 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -828,7 +828,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct usba_ep *ep = to_usba_ep(_ep);
 	struct usba_udc *udc = ep->udc;
-	struct usba_request *req = to_usba_req(_req);
+	struct usba_request *req;
 	unsigned long flags;
 	u32 status;
 
@@ -837,6 +837,16 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 
 	spin_lock_irqsave(&udc->lock, flags);
 
+	list_for_each_entry(req, &ep->queue, queue) {
+		if (&req->req == _req)
+			break;
+	}
+
+	if (&req->req != _req) {
+		spin_unlock_irqrestore(&udc->lock, flags);
+		return -EINVAL;
+	}
+
 	if (req->using_dma) {
 		/*
 		 * If this request is currently being transferred,
-- 
1.7.9.5