Currently Linux driver uses `memcpy` when reading physical memory. This
causes a kernel panic on some systems when trying to access "forbidden"
physical memory. The Linux kernel provides function
`copy_from_kernel_nofault` (previously named `probe_kernel_read`) to read
memory without crashing (and this function is used by the implementation
of `/dev/mem`).
Replace `memcpy` with `copy_from_kernel_nofault`, providing a simple
wrapper for older kernels.
While at it:
- Move the allocation of a bounce buffer outside of the while loop. As
memory is always read one page at a time, a `PAGE_SIZE` buffer is
enough.
- Use `size_t` as the type of `sz` instead of the signed type `ssize_t`.
- Replace calls to `min_t` with `if` statements which are easier to read.
- Name the number of written bytes `written` instead of `read`, in
`write_mem`.
- Remove stray spaces in various places.
By the way, the `write_mem` function was not updated to use
`copy_to_kernel_nofault`, as this function has no longer been exported
since Linux 5.8.
Fixes: https://github.com/chipsec/chipsec/issues/1094
Signed-off-by: Nicolas Iooss <nicolas.iooss_git@polytechnique.org>
Since kernel version 4.12 the GDT as seen by SGDT is only an r/o alias
mapping located at a fixed address not backed by a dedicated kernel
memory allocation. Later kernels moved it further to the "cpu entry
area", yet another alias mapping. All, just to mitigate all too easy
kernel address leaking in the name of KASLR. Anyhow, the fact that these
virtual addresses have no 'struct page' attached prevents us from using
virt_to_phys() to resolve its physical address. A full software page
table walk is required instead.
Luckily, slow_virt_to_phys() does just that. It was introduced in kernel
v3.9 so covers all affected kernels.
This fixes github issue #608.
While at it, extend the switch block to reject invalid descriptor code
arguments to not operate on uninitialized descriptor.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Reduce the visibility of my_[un]xlate_dev_mem_ptr() by making them
static, allowing the compiler to optimize their use.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
If the user provides an invalid size for the EFI variable name, we leak
the buffer used for the usercopy operation.
Fix that!
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
d_ioctl() makes excessive use of '-EFAULT' as an error code. This error
should be used for invalid userland addresses only, as otherwise
userland might be confused what the real error might be.
Use appropriate error codes instead:
- memory allocation failed: -ENOMEM
- unsupported feature: -EOPNOTSUPP
- invalid argument: -EINVAL
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
If the symbol lookup via find_symbols() fails, init_module() returns
with -1 as an error code. This is unfortunate, however, as that value
will be passed up to userland as an error code, so should rather be a
symbolic one that can be grep'ed for (-1 would be -EPERM which doesn't
seem fitting).
Use -EOPNOTSUPP instead.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
For kernels <= 2.6.33 we rely on module parameters to get the addresses
of 'page_is_ram' and 'phys_mem_access_prot'. However, if a user loads
the module without providing these arguments they will be left at zero
and lead to a NULL function pointer dereference on use.
Prevent that by always testing they're non-null.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
On kernels >= 5.10 that have CONFIG_KPROBES disabled the kprobes based
approach to get the address of kallsyms_lookup_name() won't work. While
it should still be the preferred option (as it gets us a precise
address), add another method that makes use of "code scanning" within
kernel/kallsyms.o to find the address with the help of sprint_symbol().
We scan backwards, starting at '&sprint_symbol' itself to find
kallsyms_lookup_name() by getting a symbolic representation of the
current address via sprint_symbol(). We lower the address with the
offset+1 sprint_symbol() provides us as long as we haven't found our
target function.
As kallsyms_lookup_name() is preceding sprint_symbol(), this search
should succeed. Still, to account for future code refactoring, limit the
search to 32kB at most to not run out of kernel text.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
There's no need to have a 'return 0' stub for the static calls. Simply
make them call an initialization function that will do the necessary
lookup and static call updates. This avoids the need for callers to do
the update themselves prior to making use of the functions, as the first
user will now do it for us implicitly.
Right now we would be doing it ahead of time for 'page_is_ram' users and
*always* for users of chipsec_lookup_name(). The latter is clearly
unintentional.
For kernels that have CONFIG_KPROBES disabled the kprobe based method to
get the address of kallsyms_lookup_name() will fail with '-ENOSYS'.
Returning this error code in chipsec_lookup_name() isn't appropriate as
users will wrongly use it as a function pointer as they expect only NULL
as a possible error value.
On top of fixing that, remove the ugly #ifdef'ery for 'page_is_ram'
users by providing a helper function that does "The Right Thing"
depending on the kernel version -- either use the static call or a
direct function pointer dereference.
Fix a few adjacent whitespace errors while touching the code. Also
reduce the visibility of the related symbols by making them static.
Last but not least lower the printk level to DEBUG when we fail to
register the kprobe. It's not important enough to warrant an "alert".
The already existing follow up handling of the failing symbol resolving
will provide meaningful messages already.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
While defining _GNU_SOURCE for userland code might have a legitimation,
it does nothing for kernel code. Just drop it, it's misleading.
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Several improvements
* Drop the detection of EFI through kallsyms. This detection is no
longer useful since commit 2c0c4c9fd1 ("Using CONFIG_EFI instead of
user defined HAS_EFI (#1036)") replaced `HAS_EFI` with `CONFIG_EFI`.
* Use `...-objs-y` and `...-objs-$(CONFIG_...)` logic to choose which
`cpu.asm` file is being built. This is something that other modules do
in Linux kernel source.
* Introduce `quiet_cmd_...` and `cmd_...` definition in order to use the
build system of the kernel to compile `cpu.asm` with NASM. This should
prevent issues like the one mentioned in 2210002bab ("Fixed build
error on latest Ubuntu") from happening again. Moreover this makes the
custom clean command `rm -f ${asm-path}/cpu.o` no longer necessary.
* Remove variables which become unused after these changes. Now, all
that `Makefile` needs is a kernel directory, specified with `KSRC`.
This change has been tested on a test system running Ubuntu 20.10 and on
GitHub Actions workers running Ubuntu 16.04, 18.04 and 20.04 on Linux
5.4.0-1040-azure. It has also been compile-tested on many other Linux
distributions using a work-in-progress GitHub Actions configuration
(https://github.com/fishilico/chipsec/actions/runs/661200380).
While debugging issues on Linux systems using clang CFI (Control Flow
Integrity, `CONFIG_CFI_CLANG`), it appeared to me that
`chipsec_lookup_name` could be simplified.
- Instead of using `set_fs(KERNEL_DS);` to invoke read operations,
`kernel_read` can directly be used. This function existed since at
least Linux 2.4.0
(https://elixir.bootlin.com/linux/2.4.0/source/fs/exec.c#L368) so
there should not be compatibility issues.
- Instead of allocating a 128-byte buffer with `kmalloc`, use a stack
buffer. While at it, fix a buffer overflow issue on systems with large
symbol names such as ones using clang's CFI: some lines were 142
characters long.
- Instead of matching anything on the line of `/proc/kallsyms` with
`strstr`, only do prefix-matching on the start of the symbol name.
This involves counting spaces in the line, in order to extract the
name as the 3rd item.
- Last but not least, show a clear error message if the symbol is not
found in /proc/kallsyms. This helps debugging issues.
While at it, add a comment which explains why reading `/proc/kallsyms`
does not work with Linux 5.10.
As `drivers/linux/Makefile` uses `KSRC` to define the kernel source
directory, override this variable in DKMS configuration directly.
This fixes building the Linux kernel module with dkms on Ubuntu 20.04.
It was previously possible to build Chipsec kernel module for a different Linux
kernel than the running one, and ff4a273 broke that possibility.
Bring that back just by giving a way to override the KSRC variable when calling
the Makefile.
Fix issue #1122
drivers/linux/Makefile:
* Replaced KERNEL_SRC_DIR variable with KSRC.
* Added KSRC variable which should enable successful DKMS compilation,
as opposed to bailing out with "Unable to find the Linux source tree."
This patch has been tested on Black Arch.
* Some function pointers misuse caught by PaX RAP. These workaround fix would be useful
to the vanilla kernel since the recent TigerLake support Intel CET which might have
the same complaints if the implementation can achieve the current level as PaX RAP.
* Static call is a hack to make the function pointer writable. Please keep in mind that
this hack is only for the firmware audit reason. Be cautions if you intend to do the similar
things in production.
Thanks to Spender and PaX team points out the way to fix.
Signed-off-by: Shawn C <citypw@hardenedlinux.org>
in name of security reasons. The current workaround is get the kallsyms_lookup_name()
via kprobe if the kernel >= v5.10.
Signed-off-by: Shawn C <citypw@hardenedlinux.org>
PaX implements valid_mmap_phys_addr_range in the kernel, and the
(return 1) hack in chipsec_km.c has a name collision with it.
Add another conditional for exposing the definition predicated on
whether __HAVE_ARCH_PAX_OPEN_USERLAND is defined in the kernel
headers. If it is, dont redefine the original function.
Nicolas Iooss