* [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
@ 2011-02-15 14:55 Jiri Slaby
2011-02-15 17:14 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2011-02-15 14:55 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, linux-kernel, jirislaby, Amit Shah, baoyb
tty_port_tty_get may return without any problems NULL. Handle this
case and do not oops in usb_wwan_indat_callback by dereferencing it.
The oops:
Unable to handle kernel paging request for data at address 0x000000d8
Faulting instruction address: 0xc0175b3c
Oops: Kernel access of bad area, sig: 11 [#1]
PowerPC 40x Platform
last sysfs file:
/sys/devices/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.2/usb1/idVendor
Modules linked in:
NIP: c0175b3c LR: c0175e7c CTR: c0215c90
REGS: c77f7d50 TRAP: 0300 Not tainted (2.6.37-rc5)
MSR: 00021030 <ME,CE,IR,DR> CR: 88482028 XER: 2000005f
DEAR: 000000d8, ESR: 00000000
TASK = c7141b90[1149] 'wvdial' THREAD: c2750000
GPR00: 00021030 c77f7e00 c7141b90 00000000 0000000e 00000000 0000000e c0410680
GPR08: c683db00 00000000 00000001 c03c81f8 88482028 10073ef4 ffffffb9 ffffff94
GPR16: 00000000 fde036c0 00200200 00100100 00000001 ffffff8d c34fabcc 00000000
GPR24: c71120d4 00000000 00000000 0000000e 00021030 00000000 00000000 0000000e
NIP [c0175b3c] tty_buffer_request_room+0x2c/0x194
LR [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
Call Trace:
[c77f7e00] [00000003] 0x3 (unreliable)
[c77f7e30] [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
[c77f7e60] [c0215df4] usb_wwan_indat_callback+0x164/0x170
...
References: https://bugzilla.kernel.org/show_bug.cgi?id=24582
Cc: Amit Shah <amitshah@gmx.net>
Cc: baoyb <baoyb@avit.org.cn>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/usb/serial/usb_wwan.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index b004b2a..9c014e2 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -295,12 +295,15 @@ static void usb_wwan_indat_callback(struct urb *urb)
__func__, status, endpoint);
} else {
tty = tty_port_tty_get(&port->port);
- if (urb->actual_length) {
- tty_insert_flip_string(tty, data, urb->actual_length);
- tty_flip_buffer_push(tty);
- } else
- dbg("%s: empty read urb received", __func__);
- tty_kref_put(tty);
+ if (tty) {
+ if (urb->actual_length) {
+ tty_insert_flip_string(tty, data,
+ urb->actual_length);
+ tty_flip_buffer_push(tty);
+ } else
+ dbg("%s: empty read urb received", __func__);
+ tty_kref_put(tty);
+ }
/* Resubmit urb so we continue receiving */
if (status != -ESHUTDOWN) {
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
2011-02-15 14:55 [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference Jiri Slaby
@ 2011-02-15 17:14 ` Sergei Shtylyov
2011-02-15 18:10 ` Amit Shah
2011-02-15 18:42 ` Greg KH
0 siblings, 2 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2011-02-15 17:14 UTC (permalink / raw)
To: Jiri Slaby; +Cc: gregkh, linux-usb, linux-kernel, jirislaby, Amit Shah, baoyb
Hello.
Jiri Slaby wrote:
> tty_port_tty_get may return without any problems NULL. Handle this
> case and do not oops in usb_wwan_indat_callback by dereferencing it.
> The oops:
> Unable to handle kernel paging request for data at address 0x000000d8
> Faulting instruction address: 0xc0175b3c
> Oops: Kernel access of bad area, sig: 11 [#1]
> PowerPC 40x Platform
> last sysfs file:
> /sys/devices/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.2/usb1/idVendor
> Modules linked in:
> NIP: c0175b3c LR: c0175e7c CTR: c0215c90
> REGS: c77f7d50 TRAP: 0300 Not tainted (2.6.37-rc5)
> MSR: 00021030 <ME,CE,IR,DR> CR: 88482028 XER: 2000005f
> DEAR: 000000d8, ESR: 00000000
> TASK = c7141b90[1149] 'wvdial' THREAD: c2750000
> GPR00: 00021030 c77f7e00 c7141b90 00000000 0000000e 00000000 0000000e c0410680
> GPR08: c683db00 00000000 00000001 c03c81f8 88482028 10073ef4 ffffffb9 ffffff94
> GPR16: 00000000 fde036c0 00200200 00100100 00000001 ffffff8d c34fabcc 00000000
> GPR24: c71120d4 00000000 00000000 0000000e 00021030 00000000 00000000 0000000e
> NIP [c0175b3c] tty_buffer_request_room+0x2c/0x194
> LR [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> Call Trace:
> [c77f7e00] [00000003] 0x3 (unreliable)
> [c77f7e30] [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> [c77f7e60] [c0215df4] usb_wwan_indat_callback+0x164/0x170
> ...
> References: https://bugzilla.kernel.org/show_bug.cgi?id=24582
> Cc: Amit Shah <amitshah@gmx.net>
> Cc: baoyb <baoyb@avit.org.cn>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
> drivers/usb/serial/usb_wwan.c | 15 +++++++++------
> 1 files changed, 9 insertions(+), 6 deletions(-)
> diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
> index b004b2a..9c014e2 100644
> --- a/drivers/usb/serial/usb_wwan.c
> +++ b/drivers/usb/serial/usb_wwan.c
> @@ -295,12 +295,15 @@ static void usb_wwan_indat_callback(struct urb *urb)
> __func__, status, endpoint);
> } else {
> tty = tty_port_tty_get(&port->port);
> - if (urb->actual_length) {
> - tty_insert_flip_string(tty, data, urb->actual_length);
> - tty_flip_buffer_push(tty);
> - } else
> - dbg("%s: empty read urb received", __func__);
> - tty_kref_put(tty);
> + if (tty) {
> + if (urb->actual_length) {
> + tty_insert_flip_string(tty, data,
> + urb->actual_length);
> + tty_flip_buffer_push(tty);
> + } else
> + dbg("%s: empty read urb received", __func__);
Should be {} on the *else* branch too, according to Documentation/CodingStyle...
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
2011-02-15 17:14 ` Sergei Shtylyov
@ 2011-02-15 18:10 ` Amit Shah
2011-02-15 18:36 ` Jiri Slaby
2011-02-15 18:42 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Amit Shah @ 2011-02-15 18:10 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Jiri Slaby, gregkh, linux-usb, linux-kernel, jirislaby, baoyb
On (Tue) 15 Feb 2011 [20:14:50], Sergei Shtylyov wrote:
> Hello.
>
> Jiri Slaby wrote:
>
> >tty_port_tty_get may return without any problems NULL. Handle this
> >case and do not oops in usb_wwan_indat_callback by dereferencing it.
Thanks for looking at this!
> >The oops:
> >Unable to handle kernel paging request for data at address 0x000000d8
> >Faulting instruction address: 0xc0175b3c
> >Oops: Kernel access of bad area, sig: 11 [#1]
> >PowerPC 40x Platform
> >last sysfs file:
> >/sys/devices/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.2/usb1/idVendor
> >Modules linked in:
> >NIP: c0175b3c LR: c0175e7c CTR: c0215c90
> >REGS: c77f7d50 TRAP: 0300 Not tainted (2.6.37-rc5)
> >MSR: 00021030 <ME,CE,IR,DR> CR: 88482028 XER: 2000005f
> >DEAR: 000000d8, ESR: 00000000
> >TASK = c7141b90[1149] 'wvdial' THREAD: c2750000
> >GPR00: 00021030 c77f7e00 c7141b90 00000000 0000000e 00000000 0000000e c0410680
> >GPR08: c683db00 00000000 00000001 c03c81f8 88482028 10073ef4 ffffffb9 ffffff94
> >GPR16: 00000000 fde036c0 00200200 00100100 00000001 ffffff8d c34fabcc 00000000
> >GPR24: c71120d4 00000000 00000000 0000000e 00021030 00000000 00000000 0000000e
> >NIP [c0175b3c] tty_buffer_request_room+0x2c/0x194
> >LR [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> >Call Trace:
> >[c77f7e00] [00000003] 0x3 (unreliable)
> >[c77f7e30] [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> >[c77f7e60] [c0215df4] usb_wwan_indat_callback+0x164/0x170
> >...
>
> >References: https://bugzilla.kernel.org/show_bug.cgi?id=24582
> >Cc: Amit Shah <amitshah@gmx.net>
> >Cc: baoyb <baoyb@avit.org.cn>
> >Cc: Greg Kroah-Hartman <gregkh@suse.de>
> >Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >---
> > drivers/usb/serial/usb_wwan.c | 15 +++++++++------
> > 1 files changed, 9 insertions(+), 6 deletions(-)
>
> >diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
> >index b004b2a..9c014e2 100644
> >--- a/drivers/usb/serial/usb_wwan.c
> >+++ b/drivers/usb/serial/usb_wwan.c
> >@@ -295,12 +295,15 @@ static void usb_wwan_indat_callback(struct urb *urb)
> > __func__, status, endpoint);
> > } else {
> > tty = tty_port_tty_get(&port->port);
> >- if (urb->actual_length) {
> >- tty_insert_flip_string(tty, data, urb->actual_length);
> >- tty_flip_buffer_push(tty);
> >- } else
> >- dbg("%s: empty read urb received", __func__);
> >- tty_kref_put(tty);
> >+ if (tty) {
> >+ if (urb->actual_length) {
> >+ tty_insert_flip_string(tty, data,
> >+ urb->actual_length);
> >+ tty_flip_buffer_push(tty);
> >+ } else
> >+ dbg("%s: empty read urb received", __func__);
>
> Should be {} on the *else* branch too, according to Documentation/CodingStyle...
If you're fixing that, might as well return in the 'if' above so that this
entire 'else' block becomes non-indented.
Amit
--
http://log.amitshah.net/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
2011-02-15 18:10 ` Amit Shah
@ 2011-02-15 18:36 ` Jiri Slaby
2011-02-15 18:54 ` Amit Shah
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2011-02-15 18:36 UTC (permalink / raw)
To: Amit Shah; +Cc: Sergei Shtylyov, Jiri Slaby, gregkh, linux-usb, linux-kernel
On 02/15/2011 07:10 PM, Amit Shah wrote:
> On (Tue) 15 Feb 2011 [20:14:50], Sergei Shtylyov wrote:
>> Hello.
>>
>> Jiri Slaby wrote:
>>
>>> tty_port_tty_get may return without any problems NULL. Handle this
>>> case and do not oops in usb_wwan_indat_callback by dereferencing it.
>
> Thanks for looking at this!
>
>>> The oops:
>>> Unable to handle kernel paging request for data at address 0x000000d8
>>> Faulting instruction address: 0xc0175b3c
>>> Oops: Kernel access of bad area, sig: 11 [#1]
>>> PowerPC 40x Platform
>>> last sysfs file:
>>> /sys/devices/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.2/usb1/idVendor
>>> Modules linked in:
>>> NIP: c0175b3c LR: c0175e7c CTR: c0215c90
>>> REGS: c77f7d50 TRAP: 0300 Not tainted (2.6.37-rc5)
>>> MSR: 00021030 <ME,CE,IR,DR> CR: 88482028 XER: 2000005f
>>> DEAR: 000000d8, ESR: 00000000
>>> TASK = c7141b90[1149] 'wvdial' THREAD: c2750000
>>> GPR00: 00021030 c77f7e00 c7141b90 00000000 0000000e 00000000 0000000e c0410680
>>> GPR08: c683db00 00000000 00000001 c03c81f8 88482028 10073ef4 ffffffb9 ffffff94
>>> GPR16: 00000000 fde036c0 00200200 00100100 00000001 ffffff8d c34fabcc 00000000
>>> GPR24: c71120d4 00000000 00000000 0000000e 00021030 00000000 00000000 0000000e
>>> NIP [c0175b3c] tty_buffer_request_room+0x2c/0x194
>>> LR [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
>>> Call Trace:
>>> [c77f7e00] [00000003] 0x3 (unreliable)
>>> [c77f7e30] [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
>>> [c77f7e60] [c0215df4] usb_wwan_indat_callback+0x164/0x170
>>> ...
>>
>>> References: https://bugzilla.kernel.org/show_bug.cgi?id=24582
>>> Cc: Amit Shah <amitshah@gmx.net>
>>> Cc: baoyb <baoyb@avit.org.cn>
>>> Cc: Greg Kroah-Hartman <gregkh@suse.de>
>>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>>> ---
>>> drivers/usb/serial/usb_wwan.c | 15 +++++++++------
>>> 1 files changed, 9 insertions(+), 6 deletions(-)
>>
>>> diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
>>> index b004b2a..9c014e2 100644
>>> --- a/drivers/usb/serial/usb_wwan.c
>>> +++ b/drivers/usb/serial/usb_wwan.c
>>> @@ -295,12 +295,15 @@ static void usb_wwan_indat_callback(struct urb *urb)
>>> __func__, status, endpoint);
>>> } else {
>>> tty = tty_port_tty_get(&port->port);
>>> - if (urb->actual_length) {
>>> - tty_insert_flip_string(tty, data, urb->actual_length);
>>> - tty_flip_buffer_push(tty);
>>> - } else
>>> - dbg("%s: empty read urb received", __func__);
>>> - tty_kref_put(tty);
>>> + if (tty) {
>>> + if (urb->actual_length) {
>>> + tty_insert_flip_string(tty, data,
>>> + urb->actual_length);
>>> + tty_flip_buffer_push(tty);
>>> + } else
>>> + dbg("%s: empty read urb received", __func__);
>>
>> Should be {} on the *else* branch too, according to Documentation/CodingStyle...
>
> If you're fixing that, might as well return in the 'if' above
No, the urb has to be resubmitted.
regards,
--
js
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
2011-02-15 18:36 ` Jiri Slaby
@ 2011-02-15 18:54 ` Amit Shah
0 siblings, 0 replies; 6+ messages in thread
From: Amit Shah @ 2011-02-15 18:54 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Sergei Shtylyov, Jiri Slaby, gregkh, linux-usb, linux-kernel
On (Tue) 15 Feb 2011 [19:36:28], Jiri Slaby wrote:
> > If you're fixing that, might as well return in the 'if' above
>
> No, the urb has to be resubmitted.
Ah; I meant the following, but it hardly matters..
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index b004b2a..ad89228 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -293,25 +293,24 @@ static void usb_wwan_indat_callback(struct urb *urb)
if (status) {
dbg("%s: nonzero status: %d on endpoint %02x.",
__func__, status, endpoint);
- } else {
- tty = tty_port_tty_get(&port->port);
- if (urb->actual_length) {
- tty_insert_flip_string(tty, data, urb->actual_length);
- tty_flip_buffer_push(tty);
- } else
- dbg("%s: empty read urb received", __func__);
- tty_kref_put(tty);
-
- /* Resubmit urb so we continue receiving */
- if (status != -ESHUTDOWN) {
- err = usb_submit_urb(urb, GFP_ATOMIC);
- if (err && err != -EPERM)
- printk(KERN_ERR "%s: resubmit read urb failed. "
- "(%d)", __func__, err);
- else
- usb_mark_last_busy(port->serial->dev);
- }
-
+ return;
+ }
+ tty = tty_port_tty_get(&port->port);
+ if (urb->actual_length) {
+ tty_insert_flip_string(tty, data, urb->actual_length);
+ tty_flip_buffer_push(tty);
+ } else
+ dbg("%s: empty read urb received", __func__);
+ tty_kref_put(tty);
+
+ /* Resubmit urb so we continue receiving */
+ if (status != -ESHUTDOWN) {
+ err = usb_submit_urb(urb, GFP_ATOMIC);
+ if (err && err != -EPERM)
+ printk(KERN_ERR "%s: resubmit read urb failed. "
+ "(%d)", __func__, err);
+ else
+ usb_mark_last_busy(port->serial->dev);
}
}
Amit
--
http://log.amitshah.net/
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference
2011-02-15 17:14 ` Sergei Shtylyov
2011-02-15 18:10 ` Amit Shah
@ 2011-02-15 18:42 ` Greg KH
1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2011-02-15 18:42 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Jiri Slaby, gregkh, linux-usb, linux-kernel, jirislaby, Amit Shah,
baoyb
On Tue, Feb 15, 2011 at 08:14:50PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> Jiri Slaby wrote:
>
> >tty_port_tty_get may return without any problems NULL. Handle this
> >case and do not oops in usb_wwan_indat_callback by dereferencing it.
>
> >The oops:
> >Unable to handle kernel paging request for data at address 0x000000d8
> >Faulting instruction address: 0xc0175b3c
> >Oops: Kernel access of bad area, sig: 11 [#1]
> >PowerPC 40x Platform
> >last sysfs file:
> >/sys/devices/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.2/usb1/idVendor
> >Modules linked in:
> >NIP: c0175b3c LR: c0175e7c CTR: c0215c90
> >REGS: c77f7d50 TRAP: 0300 Not tainted (2.6.37-rc5)
> >MSR: 00021030 <ME,CE,IR,DR> CR: 88482028 XER: 2000005f
> >DEAR: 000000d8, ESR: 00000000
> >TASK = c7141b90[1149] 'wvdial' THREAD: c2750000
> >GPR00: 00021030 c77f7e00 c7141b90 00000000 0000000e 00000000 0000000e c0410680
> >GPR08: c683db00 00000000 00000001 c03c81f8 88482028 10073ef4 ffffffb9 ffffff94
> >GPR16: 00000000 fde036c0 00200200 00100100 00000001 ffffff8d c34fabcc 00000000
> >GPR24: c71120d4 00000000 00000000 0000000e 00021030 00000000 00000000 0000000e
> >NIP [c0175b3c] tty_buffer_request_room+0x2c/0x194
> >LR [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> >Call Trace:
> >[c77f7e00] [00000003] 0x3 (unreliable)
> >[c77f7e30] [c0175e7c] tty_insert_flip_string_fixed_flag+0x3c/0xb0
> >[c77f7e60] [c0215df4] usb_wwan_indat_callback+0x164/0x170
> >...
>
> >References: https://bugzilla.kernel.org/show_bug.cgi?id=24582
> >Cc: Amit Shah <amitshah@gmx.net>
> >Cc: baoyb <baoyb@avit.org.cn>
> >Cc: Greg Kroah-Hartman <gregkh@suse.de>
> >Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >---
> > drivers/usb/serial/usb_wwan.c | 15 +++++++++------
> > 1 files changed, 9 insertions(+), 6 deletions(-)
>
> >diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
> >index b004b2a..9c014e2 100644
> >--- a/drivers/usb/serial/usb_wwan.c
> >+++ b/drivers/usb/serial/usb_wwan.c
> >@@ -295,12 +295,15 @@ static void usb_wwan_indat_callback(struct urb *urb)
> > __func__, status, endpoint);
> > } else {
> > tty = tty_port_tty_get(&port->port);
> >- if (urb->actual_length) {
> >- tty_insert_flip_string(tty, data, urb->actual_length);
> >- tty_flip_buffer_push(tty);
> >- } else
> >- dbg("%s: empty read urb received", __func__);
> >- tty_kref_put(tty);
> >+ if (tty) {
> >+ if (urb->actual_length) {
> >+ tty_insert_flip_string(tty, data,
> >+ urb->actual_length);
> >+ tty_flip_buffer_push(tty);
> >+ } else
> >+ dbg("%s: empty read urb received", __func__);
>
> Should be {} on the *else* branch too, according to Documentation/CodingStyle...
It's not an issue, it is fine as-is.
Jiri, thanks for the fix, I'll queue it up soon.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-02-15 18:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-15 14:55 [PATCH 1/1] USB: serial/usb_wwan, fix tty NULL dereference Jiri Slaby
2011-02-15 17:14 ` Sergei Shtylyov
2011-02-15 18:10 ` Amit Shah
2011-02-15 18:36 ` Jiri Slaby
2011-02-15 18:54 ` Amit Shah
2011-02-15 18:42 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.