11 May, 2017
1 commit
-
Pull hw lockdown support from David Howells:
"Annotation of module parameters that configure hardware resources
including ioports, iomem addresses, irq lines and dma channels.This allows a future patch to prohibit the use of such module
parameters to prevent that hardware from being abused to gain access
to the running kernel image as part of locking the kernel down under
UEFI secure boot conditions.Annotations are made by changing:
module_param(n, t, p)
module_param_named(n, v, t, p)
module_param_array(n, t, m, p)to:
module_param_hw(n, t, hwtype, p)
module_param_hw_named(n, v, t, hwtype, p)
module_param_hw_array(n, t, hwtype, m, p)where the module parameter refers to a hardware setting
hwtype specifies the type of the resource being configured. This can
be one of:ioport Module parameter configures an I/O port
iomem Module parameter configures an I/O mem address
ioport_or_iomem Module parameter could be either (runtime set)
irq Module parameter configures an I/O port
dma Module parameter configures a DMA channel
dma_addr Module parameter configures a DMA buffer address
other Module parameter configures some other valueNote that the hwtype is compile checked, but not currently stored (the
lockdown code probably won't require it). It is, however, there for
future use.A bonus is that the hwtype can also be used for grepping.
The intention is for the kernel to ignore or reject attempts to set
annotated module parameters if lockdown is enabled. This applies to
options passed on the boot command line, passed to insmod/modprobe or
direct twiddling in /sys/module/ parameter files.The module initialisation then needs to handle the parameter not being
set, by (1) giving an error, (2) probing for a value or (3) using a
reasonable default.What I can't do is just reject a module out of hand because it may
take a hardware setting in the module parameters. Some important
modules, some ipmi stuff for instance, both probe for hardware and
allow hardware to be manually specified; if the driver is aborts with
any error, you don't get any ipmi hardware.Further, trying to do this entirely in the module initialisation code
doesn't protect against sysfs twiddling.[!] Note that in and of itself, this series of patches should have no
effect on the the size of the kernel or code execution - that is
left to a patch in the next series to effect. It does mark
annotated kernel parameters with a KERNEL_PARAM_FL_HWPARAM flag in
an already existing field"* tag 'hwparam-20170420' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs: (38 commits)
Annotate hardware config module parameters in sound/pci/
Annotate hardware config module parameters in sound/oss/
Annotate hardware config module parameters in sound/isa/
Annotate hardware config module parameters in sound/drivers/
Annotate hardware config module parameters in fs/pstore/
Annotate hardware config module parameters in drivers/watchdog/
Annotate hardware config module parameters in drivers/video/
Annotate hardware config module parameters in drivers/tty/
Annotate hardware config module parameters in drivers/staging/vme/
Annotate hardware config module parameters in drivers/staging/speakup/
Annotate hardware config module parameters in drivers/staging/media/
Annotate hardware config module parameters in drivers/scsi/
Annotate hardware config module parameters in drivers/pcmcia/
Annotate hardware config module parameters in drivers/pci/hotplug/
Annotate hardware config module parameters in drivers/parport/
Annotate hardware config module parameters in drivers/net/wireless/
Annotate hardware config module parameters in drivers/net/wan/
Annotate hardware config module parameters in drivers/net/irda/
Annotate hardware config module parameters in drivers/net/hamradio/
Annotate hardware config module parameters in drivers/net/ethernet/
...
28 Apr, 2017
2 commits
-
Lockdep complains about a possible deadlock between mount and unlink
(which is technically impossible), but fixing this improves possible
future multiple-backend support, and keeps locking in the right order.The lockdep warning could be triggered by unlinking a file in the
pstore filesystem:-> #1 (&sb->s_type->i_mutex_key#14){++++++}:
lock_acquire+0xc9/0x220
down_write+0x3f/0x70
pstore_mkfile+0x1f4/0x460
pstore_get_records+0x17a/0x320
pstore_fill_super+0xa4/0xc0
mount_single+0x89/0xb0
pstore_mount+0x13/0x20
mount_fs+0xf/0x90
vfs_kern_mount+0x66/0x170
do_mount+0x190/0xd50
SyS_mount+0x90/0xd0
entry_SYSCALL_64_fastpath+0x1c/0xb1-> #0 (&psinfo->read_mutex){+.+.+.}:
__lock_acquire+0x1ac0/0x1bb0
lock_acquire+0xc9/0x220
__mutex_lock+0x6e/0x990
mutex_lock_nested+0x16/0x20
pstore_unlink+0x3f/0xa0
vfs_unlink+0xb5/0x190
do_unlinkat+0x24c/0x2a0
SyS_unlinkat+0x16/0x30
entry_SYSCALL_64_fastpath+0x1c/0xb1Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sb->s_type->i_mutex_key#14);
lock(&psinfo->read_mutex);
lock(&sb->s_type->i_mutex_key#14);
lock(&psinfo->read_mutex);Reported-by: Marta Lofstedt
Reported-by: Chris Wilson
Signed-off-by: Kees Cook
Acked-by: Namhyung Kim -
Since the vmalloc code has been removed from write_pmsg() in the commit
"5bf6d1b pstore/pmsg: drop bounce buffer", remove the unused header
vmalloc.h.Signed-off-by: Geliang Tang
Signed-off-by: Kees Cook
20 Apr, 2017
1 commit
-
When the kernel is running in secure boot mode, we lock down the kernel to
prevent userspace from modifying the running kernel image. Whilst this
includes prohibiting access to things like /dev/mem, it must also prevent
access by means of configuring driver modules in such a way as to cause a
device to access or modify the kernel image.To this end, annotate module_param* statements that refer to hardware
configuration and indicate for future reference what type of parameter they
specify. The parameter parser in the core sees this information and can
skip such parameters with an error message if the kernel is locked down.
The module initialisation then runs as normal, but just sees whatever the
default values for those parameters is.Note that we do still need to do the module initialisation because some
drivers have viable defaults set in case parameters aren't specified and
some drivers support automatic configuration (e.g. PNP or PCI) in addition
to manually coded parameters.This patch annotates drivers in fs/pstore/.
Suggested-by: Alan Cox
Signed-off-by: David Howells
Acked-by: Kees Cook
cc: Anton Vorontsov
cc: Colin Cross
cc: Tony Luck
08 Mar, 2017
19 commits
-
Nothing actually uses write_user_compat() currently, but there is no
reason to reuse the dmesg buffer. Instead, just allocate a new record
buffer, copy in from userspace, and pass it to write() as normal.Signed-off-by: Kees Cook
-
Now that write() and write_buf() are functionally identical, this removes
write_buf(), and renames write_buf_user() to write_user(). Additionally
adds sanity-checks for pstore_info's declared functions and flags at
registration time.Signed-off-by: Kees Cook
-
Removes argument list in favor of pstore record, though the user buffer
remains passed separately since it must carry the __user annotation.Signed-off-by: Kees Cook
-
As with the other API updates, this removes the long argument list in favor
of passing a single pstore recaord.Signed-off-by: Kees Cook
-
This removes the argument list for the erase() callback and replaces it
with a pointer to the backend record details to be removed.Signed-off-by: Kees Cook
-
This switches the inode-private data from carrying duplicate metadata to
keeping the record passed in during pstore_mkfile().Signed-off-by: Kees Cook
-
In preparation for handling records off to pstore_mkfile(), allocate the
record instead of reusing stack. This still always frees the record,
though, since pstore_mkfile() isn't yet keeping it.Signed-off-by: Kees Cook
-
pstore_mkfile() shouldn't have to memcpy the record contents. It can use
the existing copy instead. This adjusts the allocation lifetime management
and renames the contents variable from "data" to "buf" to assist moving to
struct pstore_record in the future.Signed-off-by: Kees Cook
-
Currently, pstore_mkfile() performs a memcpy() of the record contents,
so it can live anywhere. However, this is needlessly wasteful. In
preparation of pstore_mkfile() keeping the record contents, always
allocate a buffer for the contents.Signed-off-by: Kees Cook
-
Similar to the pstore_info read() callback, there were too many arguments.
This switches to the new struct pstore_record pointer instead. This adds
"reason" and "part" to the record structure as well.Signed-off-by: Kees Cook
-
The argument list for the pstore_read() interface is unwieldy. This changes
passes the new struct pstore_record instead. The erst backend was already
doing something similar internally.Signed-off-by: Kees Cook
-
Instead of the long list of arguments, just pass the new record struct.
Signed-off-by: Kees Cook
-
This moves the record decompression logic out to a separate function
to avoid the deep indentation.Signed-off-by: Kees Cook
-
The read/mkfile pair pass the same arguments and should be cleared
between calls. Move to a structure and wipe it after every loop.Signed-off-by: Kees Cook
-
Uncommon errors are better to get reported to dmesg so developers can
more easily figure out why pstore is unhappy with a backend attempting
to register.Signed-off-by: Kees Cook
-
Technically, it might be possible for struct pstore_info to go out of
scope after the module_put(), so report the backend name first.Signed-off-by: Kees Cook
-
When built as a module and running with update_ms >= 0, pstore will Oops
during module unload since the work timer is still running. This makes sure
the worker is stopped before unloading.Signed-off-by: Kees Cook
Cc: stable@vger.kernel.org -
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.Fixes: 109704492ef6 ("pstore: Make spinlock per zone instead of global")
Fixes: 76d5692a5803 ("pstore: Correctly initialize spinlock and flags")
Signed-off-by: Kees Cook
Cc: stable@vger.kernel.org -
The references of pstore_zbackend structures are stored into the
pointer zbackend of type struct pstore_zbackend. The pointer zbackend
can be made const as it is only dereferenced. After making this change
the pstore_zbackend structures whose references are stored into the
pointer zbackend can be made const too.File size before:
text data bss dec hex filename
4817 541 172 5530 159a fs/pstore/platform.oFile size after:
text data bss dec hex filename
4865 477 172 5514 158a fs/pstore/platform.oSigned-off-by: Bhumika Goyal
Signed-off-by: Kees Cook
25 Feb, 2017
1 commit
-
Update fs/pstore and fs/squashfs to use the updated functions from the
new LZ4 module.Link: http://lkml.kernel.org/r/1486321748-19085-5-git-send-email-4sschmid@informatik.uni-hamburg.de
Signed-off-by: Sven Schmidt
Cc: Bongkyu Kim
Cc: Rui Salvaterra
Cc: Sergey Senozhatsky
Cc: Greg Kroah-Hartman
Cc: Herbert Xu
Cc: David S. Miller
Cc: Anton Vorontsov
Cc: Colin Cross
Cc: Kees Cook
Cc: Tony Luck
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
14 Feb, 2017
2 commits
-
Instead of needing additional checks in callers for unallocated przs,
perform the check in the walker, which gives us a more universal way to
handle the situation.Signed-off-by: Kees Cook
-
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: /-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] [] dump_backtrace+0x0/0x2bc
[ 0.790780] [] show_stack+0x20/0x28
[ 0.790794] [] dump_stack+0xa4/0xcc
[ 0.790809] [] spin_dump+0xe0/0xf0
[ 0.790821] [] spin_bug+0x30/0x3c
[ 0.790834] [] do_raw_spin_lock+0x50/0x1b8
[ 0.790846] [] _raw_spin_lock_irqsave+0x54/0x6c
[ 0.790862] [] buffer_size_add+0x48/0xcc
[ 0.790875] [] persistent_ram_write+0x60/0x11c
[ 0.790888] [] ramoops_pstore_write_buf+0xd4/0x2a4
[ 0.790900] [] pstore_console_write+0xf0/0x134
[ 0.790912] [] console_unlock+0x48c/0x5e8
[ 0.790923] [] register_console+0x3b0/0x4d4
[ 0.790935] [] pstore_register+0x1a8/0x234
[ 0.790947] [] ramoops_probe+0x6b8/0x7d4
[ 0.790961] [] platform_drv_probe+0x7c/0xd0
[ 0.790972] [] driver_probe_device+0x1b4/0x3bc
[ 0.790982] [] __device_attach_driver+0xc8/0xf4
[ 0.790996] [] bus_for_each_drv+0xb4/0xe4
[ 0.791006] [] __device_attach+0xd0/0x158
[ 0.791016] [] device_initial_probe+0x24/0x30
[ 0.791026] [] bus_probe_device+0x50/0xe4
[ 0.791038] [] device_add+0x3a4/0x76c
[ 0.791051] [] of_device_add+0x74/0x84
[ 0.791062] [] of_platform_device_create_pdata+0xc0/0x100
[ 0.791073] [] of_platform_device_create+0x34/0x40
[ 0.791086] [] of_platform_default_populate_init+0x58/0x78
[ 0.791097] [] do_one_initcall+0x88/0x160
[ 0.791109] [] kernel_init_freeable+0x264/0x31c
[ 0.791123] [] kernel_init+0x18/0x11c
[ 0.791133] [] 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/0Fixes: 663deb47880f ("pstore: Allow prz to control need for locking")
Fixes: 109704492ef6 ("pstore: Make spinlock per zone instead of global")
Reported-by: Brian Norris
Signed-off-by: Kees Cook
10 Feb, 2017
1 commit
-
We'll OOPS in ramoops_get_next_prz() if the platform didn't ask for any
ftrace zones (i.e., cxt->fprzs will be NULL). Let's just skip this
entire FTRACE section if there's no 'fprzs'.Regression seen on a coreboot/depthcharge-based Chromebook.
Fixes: 2fbea82bbb89 ("pstore: Merge per-CPU ftrace records into one")
Cc: Joel Fernandes
Cc: Kees Cook
Signed-off-by: Brian Norris
Signed-off-by: Kees Cook
14 Dec, 2016
1 commit
-
Pull pstore updates from Kees Cook:
"Improvements and fixes to pstore subsystem:- add additional checks for bad platform data
- remove bounce buffer in console writer
- protect read/unlink race with a mutex
- correctly give up during dump locking failures
- increase ftrace bandwidth by splitting ftrace buffers per CPU"
* tag 'pstore-v4.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux:
ramoops: add pdata NULL check to ramoops_probe
pstore: Convert console write to use ->write_buf
pstore: Protect unlink with read_mutex
pstore: Use global ftrace filters for function trace filtering
ftrace: Provide API to use global filtering for ftrace ops
pstore: Clarify context field przs as dprzs
pstore: improve error report for failed setup
pstore: Merge per-CPU ftrace records into one
pstore: Add ftrace timestamp counter
ramoops: Split ftrace buffer space into per-CPU zones
pstore: Make ramoops_init_przs generic for other prz arrays
pstore: Allow prz to control need for locking
pstore: Warn on PSTORE_TYPE_PMSG using deprecated function
pstore: Make spinlock per zone instead of global
pstore: Actually give up during locking failure
16 Nov, 2016
11 commits
-
This adds a check for a NULL platform data, which should only be possible
if a driver incorrectly sets up a probe request without also having defined
the platform_data structure. This is based on a patch from Geliang Tang.Signed-off-by: Kees Cook
-
Maybe I'm missing something, but I don't know why it needs to copy the
input buffer to psinfo->buf and then write. Instead we can write the
input buffer directly. The only implementation that supports console
message (i.e. ramoops) already does it for ftrace messages.For the upcoming virtio backend driver, it needs to protect psinfo->buf
overwritten from console messages. If it could use ->write_buf method
instead of ->write, the problem will be solved easily.Cc: Stefan Hajnoczi
Signed-off-by: Namhyung Kim
Signed-off-by: Kees Cook -
When update_ms is set, pstore_get_records() will be called when there's
a new entry. But unlink can be called at the same time and might
contend with the open-read-close loop. Depending on the implementation
of platform driver, it may be safe or not. But I think it'd be better
to protect those race in the first place.Cc: Stefan Hajnoczi
Signed-off-by: Namhyung Kim
Signed-off-by: Kees Cook -
Currently, pstore doesn't have any filters setup for function tracing.
This has the associated overhead and may not be useful for users looking
for tracing specific set of functions.ftrace's regular function trace filtering is done writing to
tracing/set_ftrace_filter however this is not available if not requested.
In order to be able to use this feature, the support to request global
filtering introduced earlier in the series should be requested before
registering the ftrace ops. Here we do the same.Signed-off-by: Joel Fernandes
Signed-off-by: Kees Cook -
Since "przs" (persistent ram zones) is a general name in the code now, so
rename the Oops-dump zones to dprzs from przs.Based on a patch from Nobuhiro Iwamatsu.
Signed-off-by: Kees Cook
-
When setting ramoops record sizes, sometimes it's not clear which
parameters contributed to the allocation failure. This adds a per-zone
name and expands the failure reports.Signed-off-by: Kees Cook
-
Up until this patch, each of the per CPU ftrace buffers appear as a
separate ftrace-ramoops-N file. In this patch we merge all the zones into
one and populate a single ftrace-ramoops-0 file.Signed-off-by: Joel Fernandes
[kees: clarified variables names, added -ENOMEM handling]
Signed-off-by: Kees Cook -
In preparation for merging the per CPU buffers into one buffer when
we retrieve the pstore ftrace data, we store the timestamp as a
counter in the ftrace pstore record. We store the CPU number as well
if !PSTORE_CPU_IN_IP, in this case we shift the counter and may lose
ordering there but we preserve the same record size. The timestamp counter
is also racy, and not doing any locking or synchronization here results
in the benefit of lower overhead. Since we don't care much here for exact
ordering of function traces across CPUs, we don't synchronize and may lose
some counter updates but I'm ok with that.Using trace_clock() results in much lower performance so avoid using it
since we don't want accuracy in timestamp and need a rough ordering to
perform merge.Signed-off-by: Joel Fernandes
[kees: updated commit message, added comments]
Signed-off-by: Kees Cook -
If the RAMOOPS_FLAG_FTRACE_PER_CPU flag is passed to ramoops pdata, split
the ftrace space into multiple zones depending on the number of CPUs.This speeds up the performance of function tracing by about 280% in my
tests as we avoid the locking. The trade off being lesser space available
per CPU. Let the ramoops user decide which option they want based on pdata
flag.Signed-off-by: Joel Fernandes
[kees: added max_ftrace_cnt to track size, added DT logic and docs]
Signed-off-by: Kees Cook -
Currently ramoops_init_przs() is hard wired only for panic dump zone
array. In preparation for the ftrace zone array (one zone per-cpu) and pmsg
zone array, make the function more generic to be able to handle this case.Heavily based on similar work from Joel Fernandes.
Signed-off-by: Kees Cook
-
In preparation of not locking at all for certain buffers depending on if
there's contention, make locking optional depending on the initialization
of the prz.Signed-off-by: Joel Fernandes
[kees: moved locking flag into prz instead of via caller arguments]
Signed-off-by: Kees Cook
12 Nov, 2016
1 commit
-
PMSG now uses ramoops_pstore_write_buf_user() instead of ...write_buf().
Print a ratelimited warning if gets accidentally called.Signed-off-by: Joel Fernandes
[kees: adjusted commit log and added -EINVAL return]
Signed-off-by: Kees Cook