Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Thu, 19 Apr 2007 11:28:37 +0900 izumi <[EMAIL PROTECTED]> wrote:
> Russell King wrote:
>
> > NAK. This means that you change the list of ports available on the
> > machine to be limited to only those which are currently open. Utterly
> > useless for debugging, where you normally want people to dump the
> > contents of /proc/tty/driver/*.
> >
> > The original patch was better.
> >
>
>Is the original patch sufficient? or is there anything we should
> correct?
>
Would it be better to do something like
--- a/drivers/serial/serial_core.c~a
+++ a/drivers/serial/serial_core.c
@@ -1686,9 +1686,12 @@ static int uart_line_info(char *buf, str
pm_state = state->pm_state;
if (pm_state)
uart_change_pm(state, 0);
- spin_lock_irq(>lock);
- status = port->ops->get_mctrl(port);
- spin_unlock_irq(>lock);
+ status = 0;
+ if (port->info) {
+ spin_lock_irq(>lock);
+ status = port->ops->get_mctrl(port);
+ spin_unlock_irq(>lock);
+ }
if (pm_state)
uart_change_pm(state, pm_state);
mutex_unlock(>mutex);
_
so that a) we treat all uart types in the same way and b) the same problem
doesn't occur later with some other driver which is assuming an opened
device in its ->get_mctrl() handler?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
Russell King wrote: NAK. This means that you change the list of ports available on the machine to be limited to only those which are currently open. Utterly useless for debugging, where you normally want people to dump the contents of /proc/tty/driver/*. The original patch was better. Is the original patch sufficient? or is there anything we should correct? Taku Izumi <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Wed, Apr 18, 2007 at 05:21:53PM +0900, Kenji Kaneshige wrote: > > I'd imagine that other serial drivers might get upset having their > > ->get_mcrtl() called prior to being opened. Perhaps we should be fixing > > this in uart_read_proc()? > > > > I looked at other serial drivers and I could not find any other > drivers which accesses port->info in their ->get_mctrl(). This > is why we fix this problem in 8250 driver. But if there is a > possibility that other drivers accesses port->info in their > ->get_mctrl(), we should be fixing this in uart_read_proc(), as > you said. NAK. This means that you change the list of ports available on the machine to be limited to only those which are currently open. Utterly useless for debugging, where you normally want people to dump the contents of /proc/tty/driver/*. The original patch was better. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
> On Wed, 18 Apr 2007 17:21:53 +0900 Kenji Kaneshige <[EMAIL PROTECTED]> wrote: > > I'd imagine that other serial drivers might get upset having their > > ->get_mcrtl() called prior to being opened. Perhaps we should be fixing > > this in uart_read_proc()? > > > > I looked at other serial drivers and I could not find any other > drivers which accesses port->info in their ->get_mctrl(). This > is why we fix this problem in 8250 driver. But if there is a > possibility that other drivers accesses port->info in their > ->get_mctrl(), we should be fixing this in uart_read_proc(), as > you said. OK. But port->info might not be the only state which is initialised in open() which is used in get_mctrl(). > How about the following patch? We've also confirmed the problem > is fixed by it. > Thanks. Or we could just avoid calling into ->get_mctrl() if the port isn't opened. Russell? Any preferences? > > > This patch fixes the problem that uninitialized (NULL) 'info' member > of uart_port structure can be accessed if serial driver is accessed > through /proc filesystem before uart_open(), which initializes the > 'info' member', is called. > > Signed-off-by: Kenji Kaneshige <[EMAIL PROTECTED]> > Signed-off-by: Taku Izumi <[EMAIL PROTECTED]> > > --- > drivers/serial/serial_core.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6.21-rc5/drivers/serial/serial_core.c > === > --- linux-2.6.21-rc5.orig/drivers/serial/serial_core.c > +++ linux-2.6.21-rc5/drivers/serial/serial_core.c > @@ -1665,7 +1665,7 @@ static int uart_line_info(char *buf, str > unsigned int status; > int mmio, ret; > > - if (!port) > + if (!port || !port->info) > return 0; > > mmio = port->iotype >= UPIO_MEM; > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
> I'd imagine that other serial drivers might get upset having their > ->get_mcrtl() called prior to being opened. Perhaps we should be fixing > this in uart_read_proc()? > I looked at other serial drivers and I could not find any other drivers which accesses port->info in their ->get_mctrl(). This is why we fix this problem in 8250 driver. But if there is a possibility that other drivers accesses port->info in their ->get_mctrl(), we should be fixing this in uart_read_proc(), as you said. How about the following patch? We've also confirmed the problem is fixed by it. Thanks, Kenji Kaneshige This patch fixes the problem that uninitialized (NULL) 'info' member of uart_port structure can be accessed if serial driver is accessed through /proc filesystem before uart_open(), which initializes the 'info' member', is called. Signed-off-by: Kenji Kaneshige <[EMAIL PROTECTED]> Signed-off-by: Taku Izumi <[EMAIL PROTECTED]> --- drivers/serial/serial_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6.21-rc5/drivers/serial/serial_core.c === --- linux-2.6.21-rc5.orig/drivers/serial/serial_core.c +++ linux-2.6.21-rc5/drivers/serial/serial_core.c @@ -1665,7 +1665,7 @@ static int uart_line_info(char *buf, str unsigned int status; int mmio, ret; - if (!port) + if (!port || !port->info) return 0; mmio = port->iotype >= UPIO_MEM; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
I'd imagine that other serial drivers might get upset having their -get_mcrtl() called prior to being opened. Perhaps we should be fixing this in uart_read_proc()? I looked at other serial drivers and I could not find any other drivers which accesses port-info in their -get_mctrl(). This is why we fix this problem in 8250 driver. But if there is a possibility that other drivers accesses port-info in their -get_mctrl(), we should be fixing this in uart_read_proc(), as you said. How about the following patch? We've also confirmed the problem is fixed by it. Thanks, Kenji Kaneshige This patch fixes the problem that uninitialized (NULL) 'info' member of uart_port structure can be accessed if serial driver is accessed through /proc filesystem before uart_open(), which initializes the 'info' member', is called. Signed-off-by: Kenji Kaneshige [EMAIL PROTECTED] Signed-off-by: Taku Izumi [EMAIL PROTECTED] --- drivers/serial/serial_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6.21-rc5/drivers/serial/serial_core.c === --- linux-2.6.21-rc5.orig/drivers/serial/serial_core.c +++ linux-2.6.21-rc5/drivers/serial/serial_core.c @@ -1665,7 +1665,7 @@ static int uart_line_info(char *buf, str unsigned int status; int mmio, ret; - if (!port) + if (!port || !port-info) return 0; mmio = port-iotype = UPIO_MEM; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Wed, 18 Apr 2007 17:21:53 +0900 Kenji Kaneshige [EMAIL PROTECTED] wrote: I'd imagine that other serial drivers might get upset having their -get_mcrtl() called prior to being opened. Perhaps we should be fixing this in uart_read_proc()? I looked at other serial drivers and I could not find any other drivers which accesses port-info in their -get_mctrl(). This is why we fix this problem in 8250 driver. But if there is a possibility that other drivers accesses port-info in their -get_mctrl(), we should be fixing this in uart_read_proc(), as you said. OK. But port-info might not be the only state which is initialised in open() which is used in get_mctrl(). How about the following patch? We've also confirmed the problem is fixed by it. Thanks. Or we could just avoid calling into -get_mctrl() if the port isn't opened. Russell? Any preferences? This patch fixes the problem that uninitialized (NULL) 'info' member of uart_port structure can be accessed if serial driver is accessed through /proc filesystem before uart_open(), which initializes the 'info' member', is called. Signed-off-by: Kenji Kaneshige [EMAIL PROTECTED] Signed-off-by: Taku Izumi [EMAIL PROTECTED] --- drivers/serial/serial_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6.21-rc5/drivers/serial/serial_core.c === --- linux-2.6.21-rc5.orig/drivers/serial/serial_core.c +++ linux-2.6.21-rc5/drivers/serial/serial_core.c @@ -1665,7 +1665,7 @@ static int uart_line_info(char *buf, str unsigned int status; int mmio, ret; - if (!port) + if (!port || !port-info) return 0; mmio = port-iotype = UPIO_MEM; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Wed, Apr 18, 2007 at 05:21:53PM +0900, Kenji Kaneshige wrote: I'd imagine that other serial drivers might get upset having their -get_mcrtl() called prior to being opened. Perhaps we should be fixing this in uart_read_proc()? I looked at other serial drivers and I could not find any other drivers which accesses port-info in their -get_mctrl(). This is why we fix this problem in 8250 driver. But if there is a possibility that other drivers accesses port-info in their -get_mctrl(), we should be fixing this in uart_read_proc(), as you said. NAK. This means that you change the list of ports available on the machine to be limited to only those which are currently open. Utterly useless for debugging, where you normally want people to dump the contents of /proc/tty/driver/*. The original patch was better. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
Russell King wrote: NAK. This means that you change the list of ports available on the machine to be limited to only those which are currently open. Utterly useless for debugging, where you normally want people to dump the contents of /proc/tty/driver/*. The original patch was better. Is the original patch sufficient? or is there anything we should correct? Taku Izumi [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Thu, 19 Apr 2007 11:28:37 +0900 izumi [EMAIL PROTECTED] wrote:
Russell King wrote:
NAK. This means that you change the list of ports available on the
machine to be limited to only those which are currently open. Utterly
useless for debugging, where you normally want people to dump the
contents of /proc/tty/driver/*.
The original patch was better.
Is the original patch sufficient? or is there anything we should
correct?
Would it be better to do something like
--- a/drivers/serial/serial_core.c~a
+++ a/drivers/serial/serial_core.c
@@ -1686,9 +1686,12 @@ static int uart_line_info(char *buf, str
pm_state = state-pm_state;
if (pm_state)
uart_change_pm(state, 0);
- spin_lock_irq(port-lock);
- status = port-ops-get_mctrl(port);
- spin_unlock_irq(port-lock);
+ status = 0;
+ if (port-info) {
+ spin_lock_irq(port-lock);
+ status = port-ops-get_mctrl(port);
+ spin_unlock_irq(port-lock);
+ }
if (pm_state)
uart_change_pm(state, pm_state);
mutex_unlock(state-mutex);
_
so that a) we treat all uart types in the same way and b) the same problem
doesn't occur later with some other driver which is assuming an opened
device in its -get_mctrl() handler?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Tue, 17 Apr 2007 11:15:46 +0900 izumi <[EMAIL PROTECTED]> wrote:
> Hi,
>
> I encountered the following kernel panic. The cause of this problem was
> NULL pointer access in check_modem_status() in 8250.c. I confirmed
> this problem is fixed by the attached patch, but I don't know this
> is the correct fix.
>
> sadc[4378]: NaT consumption 2216203124768 [1]
> Modules linked in: binfmt_misc dm_mirror dm_mod thermal processor fan
> container button sg e100 eepro100 mii ehci_hcd ohci_hcd
>
> Pid: 4378, CPU 0, comm: sadc
> psr : 1210085a2010 ifs : 8289 ip : []
> Not tainted
> ip is at check_modem_status+0xf1/0x360
> unat: pfs : 0289 rsc : 0003
> rnat: 8000cc18 bsps: pr : 00aa6a99
> ldrs: ccv : fpsr: 0009804c8a70033f
> csd : ssd :
> b0 : a00100481fb0 b6 : a001004822e0 b7 : a00100477f20
> f6 : 1003e f7 : 0ffdba200
> f8 : 100018000 f9 : 10002a000
> f10 : 0fffdc8c0 f11 : 1003e
> r1 : a00100b9af40 r2 : 0008 r3 : a00100ad4e21
> r8 : 00bb r9 : 0001 r10 :
> r11 : a00100ad4d58 r12 : e37b7df0 r13 : e37b
> r14 : 0001 r15 : 0018 r16 : a00100ad4d6c
> r17 : r18 : r19 :
> r20 : a0010099bc88 r21 : 00bb r22 : 00bb
> r23 : c003fc0ff3fe r24 : c003fc00 r25 : 000ff3fe
> r26 : a001009b7ad0 r27 : 0001 r28 : a001009b7ad8
> r29 : r30 : a001009b7ad0 r31 : a001009b7ad0
>
> Call Trace:
> [] show_stack+0x40/0xa0
> sp=e37b7810 bsp=e37b1118
> [] show_regs+0x840/0x880
> sp=e37b79e0 bsp=e37b10c0
> [] die+0x1c0/0x2c0
> sp=e37b79e0 bsp=e37b1078
> [] die_if_kernel+0x50/0x80
> sp=e37b7a00 bsp=e37b1048
> [] ia64_fault+0x11e0/0x1300
> sp=e37b7a00 bsp=e37b0fe8
> [] ia64_leave_kernel+0x0/0x280
> sp=e37b7c20 bsp=e37b0fe8
> [] check_modem_status+0xf0/0x360
> sp=e37b7df0 bsp=e37b0fa0
> [] serial8250_get_mctrl+0x20/0xa0
> sp=e37b7df0 bsp=e37b0f80
> [] uart_read_proc+0x250/0x860
> sp=e37b7df0 bsp=e37b0ee0
> [] proc_file_read+0x1d0/0x4c0
> sp=e37b7e10 bsp=e37b0e80
> [] vfs_read+0x1b0/0x300
> sp=e37b7e20 bsp=e37b0e30
> [] sys_read+0x70/0xe0
> sp=e37b7e20 bsp=e37b0db0
> [] ia64_ret_from_syscall+0x0/0x20
> sp=e37b7e30 bsp=e37b0db0
> [] __kernel_syscall_via_break+0x0/0x20
> sp=e37b8000 bsp=e37b0db0
>
> ---
> a/drivers/serial/8250.c~fix-possible-null-pointer-access-in-8250-serial-driver
> +++ a/drivers/serial/8250.c
> @@ -1310,7 +1310,8 @@ static unsigned int check_modem_status(s
> {
> unsigned int status = serial_in(up, UART_MSR);
>
> - if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI) {
> + if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI &&
> + up->port.info != NULL) {
> if (status & UART_MSR_TERI)
> up->port.icount.rng++;
> if (status & UART_MSR_DDSR)
> _
>
I'd imagine that other serial drivers might get upset having their
->get_mcrtl() called prior to being opened. Perhaps we should be fixing
this in uart_read_proc()?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
Hi,
I encountered the following kernel panic. The cause of this problem was
NULL pointer access in check_modem_status() in 8250.c. I confirmed
this problem is fixed by the attached patch, but I don't know this
is the correct fix.
sadc[4378]: NaT consumption 2216203124768 [1]
Modules linked in: binfmt_misc dm_mirror dm_mod thermal processor fan
container button sg e100 eepro100 mii ehci_hcd ohci_hcd
Pid: 4378, CPU 0, comm: sadc
psr : 1210085a2010 ifs : 8289 ip : []
Not tainted
ip is at check_modem_status+0xf1/0x360
unat: pfs : 0289 rsc : 0003
rnat: 8000cc18 bsps: pr : 00aa6a99
ldrs: ccv : fpsr: 0009804c8a70033f
csd : ssd :
b0 : a00100481fb0 b6 : a001004822e0 b7 : a00100477f20
f6 : 1003e f7 : 0ffdba200
f8 : 100018000 f9 : 10002a000
f10 : 0fffdc8c0 f11 : 1003e
r1 : a00100b9af40 r2 : 0008 r3 : a00100ad4e21
r8 : 00bb r9 : 0001 r10 :
r11 : a00100ad4d58 r12 : e37b7df0 r13 : e37b
r14 : 0001 r15 : 0018 r16 : a00100ad4d6c
r17 : r18 : r19 :
r20 : a0010099bc88 r21 : 00bb r22 : 00bb
r23 : c003fc0ff3fe r24 : c003fc00 r25 : 000ff3fe
r26 : a001009b7ad0 r27 : 0001 r28 : a001009b7ad8
r29 : r30 : a001009b7ad0 r31 : a001009b7ad0
Call Trace:
[] show_stack+0x40/0xa0
sp=e37b7810 bsp=e37b1118
[] show_regs+0x840/0x880
sp=e37b79e0 bsp=e37b10c0
[] die+0x1c0/0x2c0
sp=e37b79e0 bsp=e37b1078
[] die_if_kernel+0x50/0x80
sp=e37b7a00 bsp=e37b1048
[] ia64_fault+0x11e0/0x1300
sp=e37b7a00 bsp=e37b0fe8
[] ia64_leave_kernel+0x0/0x280
sp=e37b7c20 bsp=e37b0fe8
[] check_modem_status+0xf0/0x360
sp=e37b7df0 bsp=e37b0fa0
[] serial8250_get_mctrl+0x20/0xa0
sp=e37b7df0 bsp=e37b0f80
[] uart_read_proc+0x250/0x860
sp=e37b7df0 bsp=e37b0ee0
[] proc_file_read+0x1d0/0x4c0
sp=e37b7e10 bsp=e37b0e80
[] vfs_read+0x1b0/0x300
sp=e37b7e20 bsp=e37b0e30
[] sys_read+0x70/0xe0
sp=e37b7e20 bsp=e37b0db0
[] ia64_ret_from_syscall+0x0/0x20
sp=e37b7e30 bsp=e37b0db0
[] __kernel_syscall_via_break+0x0/0x20
sp=e37b8000 bsp=e37b0db0
Thanks,
Taku Izumi
Fix the possible NULL pointer access in check_modem_status() in
8250.c. The check_modem_status() would access 'info' member of
uart_port structure, but it is not initialized before uart_open() is
called. The check_modem_status() can be called through
/proc/tty/driver/serial before uart_open() is called.
Signed-off-by: Kenji Kaneshige <[EMAIL PROTECTED]>
Signed-off-by: Taku Izumi <[EMAIL PROTECTED]>
---
drivers/serial/8250.c |3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.21-rc5/drivers/serial/8250.c
===
--- linux-2.6.21-rc5.orig/drivers/serial/8250.c 2007-03-26 09:14:37.0
+0900
+++ linux-2.6.21-rc5/drivers/serial/8250.c 2007-04-13 12:06:52.0
+0900
@@ -1310,7 +1310,8 @@
{
unsigned int status = serial_in(up, UART_MSR);
- if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI) {
+ if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI &&
+ up->port.info != NULL) {
if (status & UART_MSR_TERI)
up->port.icount.rng++;
if (status & UART_MSR_DDSR)
[PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
Hi,
I encountered the following kernel panic. The cause of this problem was
NULL pointer access in check_modem_status() in 8250.c. I confirmed
this problem is fixed by the attached patch, but I don't know this
is the correct fix.
sadc[4378]: NaT consumption 2216203124768 [1]
Modules linked in: binfmt_misc dm_mirror dm_mod thermal processor fan
container button sg e100 eepro100 mii ehci_hcd ohci_hcd
Pid: 4378, CPU 0, comm: sadc
psr : 1210085a2010 ifs : 8289 ip : [a00100482071]
Not tainted
ip is at check_modem_status+0xf1/0x360
unat: pfs : 0289 rsc : 0003
rnat: 8000cc18 bsps: pr : 00aa6a99
ldrs: ccv : fpsr: 0009804c8a70033f
csd : ssd :
b0 : a00100481fb0 b6 : a001004822e0 b7 : a00100477f20
f6 : 1003e f7 : 0ffdba200
f8 : 100018000 f9 : 10002a000
f10 : 0fffdc8c0 f11 : 1003e
r1 : a00100b9af40 r2 : 0008 r3 : a00100ad4e21
r8 : 00bb r9 : 0001 r10 :
r11 : a00100ad4d58 r12 : e37b7df0 r13 : e37b
r14 : 0001 r15 : 0018 r16 : a00100ad4d6c
r17 : r18 : r19 :
r20 : a0010099bc88 r21 : 00bb r22 : 00bb
r23 : c003fc0ff3fe r24 : c003fc00 r25 : 000ff3fe
r26 : a001009b7ad0 r27 : 0001 r28 : a001009b7ad8
r29 : r30 : a001009b7ad0 r31 : a001009b7ad0
Call Trace:
[a00100013940] show_stack+0x40/0xa0
sp=e37b7810 bsp=e37b1118
[a001000145a0] show_regs+0x840/0x880
sp=e37b79e0 bsp=e37b10c0
[a001000368e0] die+0x1c0/0x2c0
sp=e37b79e0 bsp=e37b1078
[a00100036a30] die_if_kernel+0x50/0x80
sp=e37b7a00 bsp=e37b1048
[a00100037c40] ia64_fault+0x11e0/0x1300
sp=e37b7a00 bsp=e37b0fe8
[a001bdc0] ia64_leave_kernel+0x0/0x280
sp=e37b7c20 bsp=e37b0fe8
[a00100482070] check_modem_status+0xf0/0x360
sp=e37b7df0 bsp=e37b0fa0
[a00100482300] serial8250_get_mctrl+0x20/0xa0
sp=e37b7df0 bsp=e37b0f80
[a00100478170] uart_read_proc+0x250/0x860
sp=e37b7df0 bsp=e37b0ee0
[a001001c16d0] proc_file_read+0x1d0/0x4c0
sp=e37b7e10 bsp=e37b0e80
[a001001394b0] vfs_read+0x1b0/0x300
sp=e37b7e20 bsp=e37b0e30
[a00100139cd0] sys_read+0x70/0xe0
sp=e37b7e20 bsp=e37b0db0
[a001bc20] ia64_ret_from_syscall+0x0/0x20
sp=e37b7e30 bsp=e37b0db0
[a0010620] __kernel_syscall_via_break+0x0/0x20
sp=e37b8000 bsp=e37b0db0
Thanks,
Taku Izumi
Fix the possible NULL pointer access in check_modem_status() in
8250.c. The check_modem_status() would access 'info' member of
uart_port structure, but it is not initialized before uart_open() is
called. The check_modem_status() can be called through
/proc/tty/driver/serial before uart_open() is called.
Signed-off-by: Kenji Kaneshige [EMAIL PROTECTED]
Signed-off-by: Taku Izumi [EMAIL PROTECTED]
---
drivers/serial/8250.c |3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.21-rc5/drivers/serial/8250.c
===
--- linux-2.6.21-rc5.orig/drivers/serial/8250.c 2007-03-26 09:14:37.0
+0900
+++ linux-2.6.21-rc5/drivers/serial/8250.c 2007-04-13 12:06:52.0
+0900
@@ -1310,7 +1310,8 @@
{
unsigned int status = serial_in(up, UART_MSR);
- if (status UART_MSR_ANY_DELTA up-ier UART_IER_MSI) {
+ if (status UART_MSR_ANY_DELTA up-ier UART_IER_MSI
+ up-port.info != NULL) {
if (status UART_MSR_TERI)
up-port.icount.rng++;
if (status UART_MSR_DDSR)
Re: [PATCH][BUG] Fix possible NULL pointer access in 8250 serial driver
On Tue, 17 Apr 2007 11:15:46 +0900 izumi [EMAIL PROTECTED] wrote:
Hi,
I encountered the following kernel panic. The cause of this problem was
NULL pointer access in check_modem_status() in 8250.c. I confirmed
this problem is fixed by the attached patch, but I don't know this
is the correct fix.
sadc[4378]: NaT consumption 2216203124768 [1]
Modules linked in: binfmt_misc dm_mirror dm_mod thermal processor fan
container button sg e100 eepro100 mii ehci_hcd ohci_hcd
Pid: 4378, CPU 0, comm: sadc
psr : 1210085a2010 ifs : 8289 ip : [a00100482071]
Not tainted
ip is at check_modem_status+0xf1/0x360
unat: pfs : 0289 rsc : 0003
rnat: 8000cc18 bsps: pr : 00aa6a99
ldrs: ccv : fpsr: 0009804c8a70033f
csd : ssd :
b0 : a00100481fb0 b6 : a001004822e0 b7 : a00100477f20
f6 : 1003e f7 : 0ffdba200
f8 : 100018000 f9 : 10002a000
f10 : 0fffdc8c0 f11 : 1003e
r1 : a00100b9af40 r2 : 0008 r3 : a00100ad4e21
r8 : 00bb r9 : 0001 r10 :
r11 : a00100ad4d58 r12 : e37b7df0 r13 : e37b
r14 : 0001 r15 : 0018 r16 : a00100ad4d6c
r17 : r18 : r19 :
r20 : a0010099bc88 r21 : 00bb r22 : 00bb
r23 : c003fc0ff3fe r24 : c003fc00 r25 : 000ff3fe
r26 : a001009b7ad0 r27 : 0001 r28 : a001009b7ad8
r29 : r30 : a001009b7ad0 r31 : a001009b7ad0
Call Trace:
[a00100013940] show_stack+0x40/0xa0
sp=e37b7810 bsp=e37b1118
[a001000145a0] show_regs+0x840/0x880
sp=e37b79e0 bsp=e37b10c0
[a001000368e0] die+0x1c0/0x2c0
sp=e37b79e0 bsp=e37b1078
[a00100036a30] die_if_kernel+0x50/0x80
sp=e37b7a00 bsp=e37b1048
[a00100037c40] ia64_fault+0x11e0/0x1300
sp=e37b7a00 bsp=e37b0fe8
[a001bdc0] ia64_leave_kernel+0x0/0x280
sp=e37b7c20 bsp=e37b0fe8
[a00100482070] check_modem_status+0xf0/0x360
sp=e37b7df0 bsp=e37b0fa0
[a00100482300] serial8250_get_mctrl+0x20/0xa0
sp=e37b7df0 bsp=e37b0f80
[a00100478170] uart_read_proc+0x250/0x860
sp=e37b7df0 bsp=e37b0ee0
[a001001c16d0] proc_file_read+0x1d0/0x4c0
sp=e37b7e10 bsp=e37b0e80
[a001001394b0] vfs_read+0x1b0/0x300
sp=e37b7e20 bsp=e37b0e30
[a00100139cd0] sys_read+0x70/0xe0
sp=e37b7e20 bsp=e37b0db0
[a001bc20] ia64_ret_from_syscall+0x0/0x20
sp=e37b7e30 bsp=e37b0db0
[a0010620] __kernel_syscall_via_break+0x0/0x20
sp=e37b8000 bsp=e37b0db0
---
a/drivers/serial/8250.c~fix-possible-null-pointer-access-in-8250-serial-driver
+++ a/drivers/serial/8250.c
@@ -1310,7 +1310,8 @@ static unsigned int check_modem_status(s
{
unsigned int status = serial_in(up, UART_MSR);
- if (status UART_MSR_ANY_DELTA up-ier UART_IER_MSI) {
+ if (status UART_MSR_ANY_DELTA up-ier UART_IER_MSI
+ up-port.info != NULL) {
if (status UART_MSR_TERI)
up-port.icount.rng++;
if (status UART_MSR_DDSR)
_
I'd imagine that other serial drivers might get upset having their
-get_mcrtl() called prior to being opened. Perhaps we should be fixing
this in uart_read_proc()?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
