* [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
@ 2014-12-01 9:12 Mika Westerberg
2014-12-04 21:01 ` Robert Jarzmik
2014-12-23 13:49 ` Mika Westerberg
0 siblings, 2 replies; 6+ messages in thread
From: Mika Westerberg @ 2014-12-01 9:12 UTC (permalink / raw)
To: linux-spi
Cc: Mark Brown, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Martin Oldfield, Mika Westerberg, linux-kernel
Once the current message is finished, the driver notifies SPI core about
this by calling spi_finalize_current_message(). This function queues next
message to be transferred. If there are more messages in the queue, it is
possible that the driver is asked to transfer the next message at this
point.
When spi_finalize_current_message() returns the driver clears the
drv_data->cur_chip pointer to NULL. The problem is that if the driver
already started the next message clearing drv_data->cur_chip will cause
NULL pointer dereference which crashes the kernel like:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
IP: [<ffffffffa0022bc8>] cs_deassert+0x18/0x70 [spi_pxa2xx_platform]
PGD 78bb8067 PUD 37712067 PMD 0
Oops: 0000 [#1] SMP
Modules linked in:
CPU: 1 PID: 11 Comm: ksoftirqd/1 Tainted: G O 3.18.0-rc4-mjo #5
Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS MNW2CRB1.X64.0071.R30.1408131301 08/13/2014
task: ffff880077f9f290 ti: ffff88007a820000 task.ti: ffff88007a820000
RIP: 0010:[<ffffffffa0022bc8>] [<ffffffffa0022bc8>] cs_deassert+0x18/0x70 [spi_pxa2xx_platform]
RSP: 0018:ffff88007a823d08 EFLAGS: 00010202
RAX: 0000000000000008 RBX: ffff8800379a4430 RCX: 0000000000000026
RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffff8800379a4430
RBP: ffff88007a823d18 R08: 00000000ffffffff R09: 000000007a9bc65a
R10: 000000000000028f R11: 0000000000000005 R12: ffff880070123e98
R13: ffff880070123de8 R14: 0000000000000100 R15: ffffc90004888000
FS: 0000000000000000(0000) GS:ffff880079a80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000048 CR3: 000000007029b000 CR4: 00000000001007e0
Stack:
ffff88007a823d58 ffff8800379a4430 ffff88007a823d48 ffffffffa0022c89
0000000000000000 ffff8800379a4430 0000000000000000 0000000000000006
ffff88007a823da8 ffffffffa0023be0 ffff88007a823dd8 ffffffff81076204
Call Trace:
[<ffffffffa0022c89>] giveback+0x69/0xa0 [spi_pxa2xx_platform]
[<ffffffffa0023be0>] pump_transfers+0x710/0x740 [spi_pxa2xx_platform]
[<ffffffff81076204>] ? pick_next_task_fair+0x744/0x830
[<ffffffff81049679>] tasklet_action+0xa9/0xe0
[<ffffffff81049a0e>] __do_softirq+0xee/0x280
[<ffffffff81049bc0>] run_ksoftirqd+0x20/0x40
[<ffffffff810646df>] smpboot_thread_fn+0xff/0x1b0
[<ffffffff810645e0>] ? SyS_setgroups+0x150/0x150
[<ffffffff81060f9d>] kthread+0xcd/0xf0
[<ffffffff81060ed0>] ? kthread_create_on_node+0x180/0x180
[<ffffffff8187a82c>] ret_from_fork+0x7c/0xb0
Fix this by clearing drv_data->cur_chip before we call spi_finalize_current_message().
Reported-by: Martin Oldfield <m@mjoldfield.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/spi/spi-pxa2xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 9e9e0f971e6c..d95656d05eb6 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -402,8 +402,8 @@ static void giveback(struct driver_data *drv_data)
cs_deassert(drv_data);
}
- spi_finalize_current_message(drv_data->master);
drv_data->cur_chip = NULL;
+ spi_finalize_current_message(drv_data->master);
}
static void reset_sccr1(struct driver_data *drv_data)
--
2.1.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
2014-12-01 9:12 [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message Mika Westerberg
@ 2014-12-04 21:01 ` Robert Jarzmik
2014-12-05 8:24 ` Mika Westerberg
2014-12-23 13:49 ` Mika Westerberg
1 sibling, 1 reply; 6+ messages in thread
From: Robert Jarzmik @ 2014-12-04 21:01 UTC (permalink / raw)
To: Mika Westerberg
Cc: linux-spi, Mark Brown, Daniel Mack, Haojian Zhuang,
Martin Oldfield, linux-kernel
Mika Westerberg <mika.westerberg@linux.intel.com> writes:
> Once the current message is finished, the driver notifies SPI core about
> this by calling spi_finalize_current_message(). This function queues next
> message to be transferred. If there are more messages in the queue, it is
> possible that the driver is asked to transfer the next message at this
> point.
>
> When spi_finalize_current_message() returns the driver clears the
> drv_data->cur_chip pointer to NULL. The problem is that if the driver
> already started the next message clearing drv_data->cur_chip will cause
> NULL pointer dereference which crashes the kernel like:
..zip..
> Fix this by clearing drv_data->cur_chip before we call
> spi_finalize_current_message().
So with your change, we have :
drv_data->cur_chip = NULL;
spi_finalize_current_message(drv_data->master);
In that case, if spi_finalize_current_message() queues another message, upon
this next message completion, won't giveback() be called, and dereference
cur_chip as well ?
Cheers.
--
Robert
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
2014-12-04 21:01 ` Robert Jarzmik
@ 2014-12-05 8:24 ` Mika Westerberg
2014-12-05 18:36 ` Robert Jarzmik
0 siblings, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2014-12-05 8:24 UTC (permalink / raw)
To: Robert Jarzmik
Cc: linux-spi, Mark Brown, Daniel Mack, Haojian Zhuang,
Martin Oldfield, linux-kernel
On Thu, Dec 04, 2014 at 10:01:06PM +0100, Robert Jarzmik wrote:
> Mika Westerberg <mika.westerberg@linux.intel.com> writes:
>
> > Once the current message is finished, the driver notifies SPI core about
> > this by calling spi_finalize_current_message(). This function queues next
> > message to be transferred. If there are more messages in the queue, it is
> > possible that the driver is asked to transfer the next message at this
> > point.
> >
> > When spi_finalize_current_message() returns the driver clears the
> > drv_data->cur_chip pointer to NULL. The problem is that if the driver
> > already started the next message clearing drv_data->cur_chip will cause
> > NULL pointer dereference which crashes the kernel like:
> ..zip..
> > Fix this by clearing drv_data->cur_chip before we call
> > spi_finalize_current_message().
>
> So with your change, we have :
> drv_data->cur_chip = NULL;
> spi_finalize_current_message(drv_data->master);
>
> In that case, if spi_finalize_current_message() queues another message, upon
> this next message completion, won't giveback() be called, and dereference
> cur_chip as well ?
When the next message is started pxa2xx_spi_transfer_one_message() gets
called and that will set cur_chip again.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
2014-12-05 8:24 ` Mika Westerberg
@ 2014-12-05 18:36 ` Robert Jarzmik
0 siblings, 0 replies; 6+ messages in thread
From: Robert Jarzmik @ 2014-12-05 18:36 UTC (permalink / raw)
To: Mika Westerberg
Cc: linux-spi, Mark Brown, Daniel Mack, Haojian Zhuang,
Martin Oldfield, linux-kernel
Mika Westerberg <mika.westerberg@linux.intel.com> writes:
> On Thu, Dec 04, 2014 at 10:01:06PM +0100, Robert Jarzmik wrote:
>> So with your change, we have :
>> drv_data->cur_chip = NULL;
>> spi_finalize_current_message(drv_data->master);
>>
>> In that case, if spi_finalize_current_message() queues another message, upon
>> this next message completion, won't giveback() be called, and dereference
>> cur_chip as well ?
>
> When the next message is started pxa2xx_spi_transfer_one_message() gets
> called and that will set cur_chip again.
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cheers.
--
Robert
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
2014-12-01 9:12 [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message Mika Westerberg
2014-12-04 21:01 ` Robert Jarzmik
@ 2014-12-23 13:49 ` Mika Westerberg
2014-12-23 17:34 ` Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2014-12-23 13:49 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Martin Oldfield, linux-kernel
On Mon, Dec 01, 2014 at 11:12:46AM +0200, Mika Westerberg wrote:
> Once the current message is finished, the driver notifies SPI core about
> this by calling spi_finalize_current_message(). This function queues next
> message to be transferred. If there are more messages in the queue, it is
> possible that the driver is asked to transfer the next message at this
> point.
>
> When spi_finalize_current_message() returns the driver clears the
> drv_data->cur_chip pointer to NULL. The problem is that if the driver
> already started the next message clearing drv_data->cur_chip will cause
> NULL pointer dereference which crashes the kernel like:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
> IP: [<ffffffffa0022bc8>] cs_deassert+0x18/0x70 [spi_pxa2xx_platform]
> PGD 78bb8067 PUD 37712067 PMD 0
> Oops: 0000 [#1] SMP
> Modules linked in:
> CPU: 1 PID: 11 Comm: ksoftirqd/1 Tainted: G O 3.18.0-rc4-mjo #5
> Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS MNW2CRB1.X64.0071.R30.1408131301 08/13/2014
> task: ffff880077f9f290 ti: ffff88007a820000 task.ti: ffff88007a820000
> RIP: 0010:[<ffffffffa0022bc8>] [<ffffffffa0022bc8>] cs_deassert+0x18/0x70 [spi_pxa2xx_platform]
> RSP: 0018:ffff88007a823d08 EFLAGS: 00010202
> RAX: 0000000000000008 RBX: ffff8800379a4430 RCX: 0000000000000026
> RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffff8800379a4430
> RBP: ffff88007a823d18 R08: 00000000ffffffff R09: 000000007a9bc65a
> R10: 000000000000028f R11: 0000000000000005 R12: ffff880070123e98
> R13: ffff880070123de8 R14: 0000000000000100 R15: ffffc90004888000
> FS: 0000000000000000(0000) GS:ffff880079a80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000048 CR3: 000000007029b000 CR4: 00000000001007e0
> Stack:
> ffff88007a823d58 ffff8800379a4430 ffff88007a823d48 ffffffffa0022c89
> 0000000000000000 ffff8800379a4430 0000000000000000 0000000000000006
> ffff88007a823da8 ffffffffa0023be0 ffff88007a823dd8 ffffffff81076204
> Call Trace:
> [<ffffffffa0022c89>] giveback+0x69/0xa0 [spi_pxa2xx_platform]
> [<ffffffffa0023be0>] pump_transfers+0x710/0x740 [spi_pxa2xx_platform]
> [<ffffffff81076204>] ? pick_next_task_fair+0x744/0x830
> [<ffffffff81049679>] tasklet_action+0xa9/0xe0
> [<ffffffff81049a0e>] __do_softirq+0xee/0x280
> [<ffffffff81049bc0>] run_ksoftirqd+0x20/0x40
> [<ffffffff810646df>] smpboot_thread_fn+0xff/0x1b0
> [<ffffffff810645e0>] ? SyS_setgroups+0x150/0x150
> [<ffffffff81060f9d>] kthread+0xcd/0xf0
> [<ffffffff81060ed0>] ? kthread_create_on_node+0x180/0x180
> [<ffffffff8187a82c>] ret_from_fork+0x7c/0xb0
>
> Fix this by clearing drv_data->cur_chip before we call spi_finalize_current_message().
>
> Reported-by: Martin Oldfield <m@mjoldfield.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Mark, any comments on this?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message
2014-12-23 13:49 ` Mika Westerberg
@ 2014-12-23 17:34 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-12-23 17:34 UTC (permalink / raw)
To: Mika Westerberg
Cc: linux-spi, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Martin Oldfield, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 207 bytes --]
On Tue, Dec 23, 2014 at 03:49:17PM +0200, Mika Westerberg wrote:
> Mark, any comments on this?
Please don't send contentless pings, they just add more e-mail. Resend
if you think something has been lost.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-23 17:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-01 9:12 [PATCH] spi/pxa2xx: Clear cur_chip pointer before starting next message Mika Westerberg
2014-12-04 21:01 ` Robert Jarzmik
2014-12-05 8:24 ` Mika Westerberg
2014-12-05 18:36 ` Robert Jarzmik
2014-12-23 13:49 ` Mika Westerberg
2014-12-23 17:34 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox