New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 719005 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

lockdep splat from pstore on kernel 4.4

Project Member Reported by diand...@chromium.org, May 5 2017

Issue description

When I turn on lockdep in 4.4 with these three patches:

remote:   https://chromium-review.googlesource.com/497929 TEST-ONLY: Hack out some sort of lockdep thing in sock.h        
remote:   https://chromium-review.googlesource.com/497930 WIP: OPP: Fix compiler warnings when RCU debugging is on        
remote:   https://chromium-review.googlesource.com/497931 TEST-ONLY: CHROMIUM: config: Turn on lockdep        

I get a splat at bootup:

[    0.267264] the code is fine but needs lockdep annotation.
[    0.267266] turning off the locking correctness validator.
[    0.267272] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.4.64 #119
[    0.267273] Hardware name: Google Kevin (DT)
[    0.267276] Call trace:
[    0.267283] [<ffffffc000208ae4>] dump_backtrace+0x0/0x160
[    0.267287] [<ffffffc000208d74>] show_stack+0x20/0x28
[    0.267292] [<ffffffc00051a370>] dump_stack+0xb4/0xf0
[    0.267297] [<ffffffc00028656c>] __lock_acquire+0x880/0x1a74
[    0.267301] [<ffffffc000287e3c>] lock_acquire+0x248/0x280
[    0.267307] [<ffffffc0009d86e4>] _raw_spin_lock_irqsave+0x5c/0x74
[    0.267313] [<ffffffc00048f130>] buffer_size_add+0x30/0x78
[    0.267318] [<ffffffc00048f5fc>] persistent_ram_write+0x50/0xec
[    0.267321] [<ffffffc00048de40>] ramoops_pstore_write_buf+0x8c/0x200
[    0.267325] [<ffffffc00048d2e8>] pstore_console_write+0xe4/0x12c
[    0.267331] [<ffffffc000291d98>] call_console_drivers.constprop.24+0x1c8/0x1f8
[    0.267335] [<ffffffc000293970>] console_unlock+0x3e4/0x4b0
[    0.267339] [<ffffffc000294728>] register_console+0x280/0x36c
[    0.267343] [<ffffffc00048dbf8>] pstore_register+0x154/0x1c0
[    0.267346] [<ffffffc00048ef8c>] ramoops_probe+0x518/0x664
[    0.267352] [<ffffffc000656ea8>] platform_drv_probe+0x60/0xac
[    0.267356] [<ffffffc000654a24>] driver_probe_device+0x120/0x2bc
[    0.267359] [<ffffffc000654d44>] __device_attach_driver+0x90/0xa0
[    0.267363] [<ffffffc000652ab8>] bus_for_each_drv+0x98/0xc8
[    0.267366] [<ffffffc000654854>] __device_attach+0xb0/0x130
[    0.267370] [<ffffffc000654ed0>] device_initial_probe+0x24/0x30
[    0.267373] [<ffffffc000653d94>] bus_probe_device+0x38/0x9c
[    0.267376] [<ffffffc000651960>] device_add+0x2d4/0x520
[    0.267383] [<ffffffc0007e9cac>] of_device_add+0x44/0x50
[    0.267386] [<ffffffc0007ea54c>] of_platform_device_create_pdata+0x9c/0xdc
[    0.267389] [<ffffffc0007ea81c>] of_platform_bus_create+0x250/0x2f8
[    0.267392] [<ffffffc0007eaa0c>] of_platform_populate+0x78/0xb4
[    0.267397] [<ffffffc000e02d58>] arm64_device_init+0x2c/0x48
[    0.267400] [<ffffffc000201100>] do_one_initcall+0x194/0x1b0
[    0.267404] [<ffffffc000e00c18>] kernel_init_freeable+0x224/0x2dc
[    0.267408] [<ffffffc0009d1900>] kernel_init+0x20/0xe4
[    0.267412] [<ffffffc000203dd0>] ret_from_fork+0x10/0x40

---

It appears that upstream has already fixed this.  I'll post patches shortly.
 
remote:   https://chromium-review.googlesource.com/497910 Revert "FROMLIST: pstore: unconditionally initialize spinlock and flags"        
remote:   https://chromium-review.googlesource.com/497911 UPSTREAM: pstore: Correctly initialize spinlock and flags        
remote:   https://chromium-review.googlesource.com/497912 UPSTREAM: pstore: Use dynamic spinlock initializer        

Project Member

Comment 2 by bugdroid1@chromium.org, May 7 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/04bee81f1781af613b560d1a68deab732fc9d7e0

commit 04bee81f1781af613b560d1a68deab732fc9d7e0
Author: Douglas Anderson <dianders@chromium.org>
Date: Sun May 07 02:52:21 2017

Revert "FROMLIST: pstore: unconditionally initialize spinlock and flags"

This reverts commit e30ab9304dd7082d6f5330e713bfc9ba1cd5616d.

We'll pick the upstream version.

CQ-DEPEND=CL:Ibef7b0b768ea10869cd3c0242288fafb37687869
BUG= chromium:719005 
TEST=No more splat at bootup

Change-Id: Ia46080310f063abf84e82feb003b8582b4f20015
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/497910
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>

[modify] https://crrev.com/04bee81f1781af613b560d1a68deab732fc9d7e0/fs/pstore/ram_core.c

Project Member

Comment 3 by bugdroid1@chromium.org, May 7 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/63a038ba9e2afcc11c26639ae2bf2c9cd96136ed

commit 63a038ba9e2afcc11c26639ae2bf2c9cd96136ed
Author: Kees Cook <keescook@chromium.org>
Date: Sun May 07 02:52:22 2017

UPSTREAM: pstore: Correctly initialize spinlock and flags

The ram backend wasn't always initializing its spinlock correctly. Since
it was coming from kzalloc memory, though, it was harmless on
architectures that initialize unlocked spinlocks to 0 (at least x86 and
ARM). This also fixes a possibly ignored flag setting too.

When running under CONFIG_DEBUG_SPINLOCK, the following Oops was visible:

[    0.760836] persistent_ram: found existing buffer, size 29988, start 29988
[    0.765112] persistent_ram: found existing buffer, size 30105, start 30105
[    0.769435] persistent_ram: found existing buffer, size 118542, start 118542
[    0.785960] persistent_ram: found existing buffer, size 0, start 0
[    0.786098] persistent_ram: found existing buffer, size 0, start 0
[    0.786131] pstore: using zlib compression
[    0.790716] BUG: spinlock bad magic on CPU#0, swapper/0/1
[    0.790729]  lock: 0xffffffc0d1ca9bb0, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
[    0.790742] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.10.0-rc2+ #913
[    0.790747] Hardware name: Google Kevin (DT)
[    0.790750] Call trace:
[    0.790768] [<ffffff900808ae88>] dump_backtrace+0x0/0x2bc
[    0.790780] [<ffffff900808b164>] show_stack+0x20/0x28
[    0.790794] [<ffffff9008460ee0>] dump_stack+0xa4/0xcc
[    0.790809] [<ffffff9008113cfc>] spin_dump+0xe0/0xf0
[    0.790821] [<ffffff9008113d3c>] spin_bug+0x30/0x3c
[    0.790834] [<ffffff9008113e28>] do_raw_spin_lock+0x50/0x1b8
[    0.790846] [<ffffff9008a2d2ec>] _raw_spin_lock_irqsave+0x54/0x6c
[    0.790862] [<ffffff90083ac3b4>] buffer_size_add+0x48/0xcc
[    0.790875] [<ffffff90083acb34>] persistent_ram_write+0x60/0x11c
[    0.790888] [<ffffff90083aab1c>] ramoops_pstore_write_buf+0xd4/0x2a4
[    0.790900] [<ffffff90083a9d3c>] pstore_console_write+0xf0/0x134
[    0.790912] [<ffffff900811c304>] console_unlock+0x48c/0x5e8
[    0.790923] [<ffffff900811da18>] register_console+0x3b0/0x4d4
[    0.790935] [<ffffff90083aa7d0>] pstore_register+0x1a8/0x234
[    0.790947] [<ffffff90083ac250>] ramoops_probe+0x6b8/0x7d4
[    0.790961] [<ffffff90085ca548>] platform_drv_probe+0x7c/0xd0
[    0.790972] [<ffffff90085c76ac>] driver_probe_device+0x1b4/0x3bc
[    0.790982] [<ffffff90085c7ac8>] __device_attach_driver+0xc8/0xf4
[    0.790996] [<ffffff90085c4bfc>] bus_for_each_drv+0xb4/0xe4
[    0.791006] [<ffffff90085c7414>] __device_attach+0xd0/0x158
[    0.791016] [<ffffff90085c7b18>] device_initial_probe+0x24/0x30
[    0.791026] [<ffffff90085c648c>] bus_probe_device+0x50/0xe4
[    0.791038] [<ffffff90085c35b8>] device_add+0x3a4/0x76c
[    0.791051] [<ffffff90087d0e84>] of_device_add+0x74/0x84
[    0.791062] [<ffffff90087d19b8>] of_platform_device_create_pdata+0xc0/0x100
[    0.791073] [<ffffff90087d1a2c>] of_platform_device_create+0x34/0x40
[    0.791086] [<ffffff900903c910>] of_platform_default_populate_init+0x58/0x78
[    0.791097] [<ffffff90080831fc>] do_one_initcall+0x88/0x160
[    0.791109] [<ffffff90090010ac>] kernel_init_freeable+0x264/0x31c
[    0.791123] [<ffffff9008a25bd0>] kernel_init+0x18/0x11c
[    0.791133] [<ffffff9008082ec0>] ret_from_fork+0x10/0x50
[    0.793717] console [pstore-1] enabled
[    0.797845] pstore: Registered ramoops as persistent store backend
[    0.804647] ramoops: attached 0x100000@0xf7edc000, ecc: 0/0

BUG= chromium:719005 
TEST=Build and boot

Change-Id: Ifefe5b731bddaee9edf7562a153f3636424df299
Fixes: 663deb47880f ("pstore: Allow prz to control need for locking")
Fixes: 109704492ef6 ("pstore: Make spinlock per zone instead of global")
Reported-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 76d5692a58031696e282384cbd893832bc92bd76)
Reviewed-on: https://chromium-review.googlesource.com/497911
Reviewed-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/63a038ba9e2afcc11c26639ae2bf2c9cd96136ed/fs/pstore/ram_core.c

Project Member

Comment 4 by bugdroid1@chromium.org, May 7 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/990177cd78f3b4fc39c2c6e563f8563e225e1ec7

commit 990177cd78f3b4fc39c2c6e563f8563e225e1ec7
Author: Kees Cook <keescook@chromium.org>
Date: Sun May 07 02:52:24 2017

UPSTREAM: pstore: Use dynamic spinlock initializer

The per-prz spinlock should be using the dynamic initializer so that
lockdep can correctly track it. Without this, under lockdep, we get a
warning at boot that the lock is in non-static memory.

BUG= chromium:719005 
TEST=Build and boot

Change-Id: Ibef7b0b768ea10869cd3c0242288fafb37687869
Fixes: 109704492ef6 ("pstore: Make spinlock per zone instead of global")
Fixes: 76d5692a5803 ("pstore: Correctly initialize spinlock and flags")
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit e9a330c4289f2ba1ca4bf98c2b430ab165a8931b)
Reviewed-on: https://chromium-review.googlesource.com/497912
Reviewed-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/990177cd78f3b4fc39c2c6e563f8563e225e1ec7/fs/pstore/ram_core.c

Status: Fixed (was: Started)

Comment 6 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment