On 16/10/2022 22:07, Günther Noack wrote:
On Sun, Oct 16, 2022 at 08:11:31PM +0200, Günther Noack wrote:
On Thu, Oct 13, 2022 at 09:35:24AM -0700, Nathan Chancellor wrote:
Hi Mickaël and Günther,
On Mon, Oct 10, 2022 at 12:35:31PM +0200, Mickaël Salaün wrote:
Thanks Günther! This series looks good and is now in -next with some minor
cosmetic comment changes.
Nathan, could you please confirm that this series work for you?
First of all, let me apologize for the delay in response. I am just now
getting back online after a week long vacation, which was definitely
poorly timed with the merge window :/
Unfortunately, with this series applied on top of commit e8bc52cb8df8
("Merge tag 'driver-core-6.1-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core") as
indicated by the base commit information at the bottom of the cover
letter, I can still reproduce the original crash I reported. What is
even more odd is that I should be using the exact same tool versions
that Günther is, as I am also using Arch Linux as my distribution.
I have attached the exact .config that the build system produced after
my build, just in case there is something else in our environment that
could be causing difficulties in reproducing.
For what it is worth, I can reproduce this in a fresh Arch Linux
container, which should hopefully remove most environment concerns.
$ podman run \
--interactive \
--tty \
--rm
--volume .../linux-next:/linux-next \
--workdir /linux-next \
docker.io/archlinux:base-devel
# pacman -Syyu --noconfirm \
aarch64-linux-gnu-gcc \
bc \
git \
pahole \
python3 \
qemu-system-aarch64
...
# aarch64-linux-gnu-gcc --version | head -1
aarch64-linux-gnu-gcc (GCC) 12.2.0
# aarch64-linux-gnu-as --version | head -1
GNU assembler (GNU Binutils) 2.39
# qemu-system-aarch64 --version | head -1
QEMU emulator version 7.1.0
# git log --first-parent --oneline e8bc52cb8df8^..
5622ae16a601 landlock: Document Landlock's file truncation support
6c8a1dadeae1 samples/landlock: Extend sample tool to support LANDLOCK_ACCESS_FS_TRUNCATE
d19c9ba61c75 selftests/landlock: Test ftruncate on FDs created by memfd_create(2)
bf5e5018edb5 selftests/landlock: Test FD passing from restricted to unrestricted processes
4a7f660a22b2 selftests/landlock: Locally define __maybe_unused
1a9015ef7014 selftests/landlock: Test open() and ftruncate() in multiple scenarios
79bb219d0b7c selftests/landlock: Test file truncation support
dd3d0e23543e landlock: Support file truncation
dcade986e070 landlock: Document init_layer_masks() helper
873afb813b11 landlock: Refactor check_access_path_dual() into is_access_to_paths_allowed()
cdda4d440c96 security: Create file_truncate hook from path_truncate hook
e8bc52cb8df8 Merge tag 'driver-core-6.1-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
# mkdir build
# mv .config build
# mv rootfs.cpio build
# make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build Image.gz
# qemu-system-aarch64 \
-machine virt,gic-version=max,virtualization=true \
-cpu max,pauth-impdef=true \
-kernel build/arch/arm64/boot/Image.gz \
-append "console=ttyAMA0 earlycon" \
-display none \
-initrd build/rootfs.cpio \
-m 512m \
-nodefaults \
-no-reboot \
-serial mon:stdio
...
[ 0.000000] Linux version 6.0.0-08005-g5622ae16a601 (root@82bc572c5e5f) (aarch64-linux-gnu-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.39) #1 SMP Thu Oct 13 16:30:30 UTC 2022
...
[ 0.491767] Trying to unpack rootfs image as initramfs...
[ 0.494156] Unable to handle kernel paging request at virtual address ffff00000851036a
[ 0.494389] Mem abort info:
[ 0.494466] ESR = 0x0000000097c0c061
[ 0.494601] EC = 0x25: DABT (current EL), IL = 32 bits
[ 0.494756] SET = 0, FnV = 0
[ 0.494957] EA = 0, S1PTW = 0
[ 0.495070] FSC = 0x21: alignment fault
^^^^^^^^^^^^^^^
It's a memory alignment bug:
Problem
-------
When the LSM framework determines the positions of the security blobs
from their sizes, is just concatenates the blobs directly, which can
lead to later blobs being misaligned if an earlier security blob has a
misaligned size.
In this patch set, we introduced a pretty short (u16) file security
blob, and that shifted the alignment of the AppArmor blob to an
unaligned address, causing this Oops. (The other existing LSM blobs
just happen to have sizes where it was not a problem before.)
Interesting
Proposed fix
------------
I think the LSM framework should ensure that security blobs are
pointer-aligned.
The LSM framework takes the role of a memory allocator here, and
memory allocators should normally return aligned addresses, in my
understanding. -- It seems reasonable for AppArmor to make that
assumption.
The proposed one-line fix is: Change lsm_set_blob_size() in
security/security.c, where the positions of the individual security
blobs are calculated, so that each allocated blob is aligned to a
pointer size boundary.
if (*need > 0) {
*lbs = ALIGN(*lbs, sizeof(void *)); // NEW
offset = *lbs;
*lbs += *need;
*need = offset;
}
This looks good to me. This fix should be part of patch 4/11 since it
only affects Landlock for now.
Alternatives considered
-----------------------
The obvious alternative fix would be to fix it locally to Landlock and
pad struct landlock_file_security to have the multiple of a pointer
size.
Tradeoffs:
+ This approach would also fix the Oops
- Problem could happen again with another security blob in the
future, and the fact that is happens only with multiple LSMs
enabled makes that problem difficult to spot during LSM
development.
Paul, does that proposed fix (to make lsm_set_blob_size() calculate
aligned offsets) make sense to you?
(Also +CC: Thomas Gleixner, FYI, because you have authored
lsm_set_blob_size() originally - do you have opinions on this?)
Thanks,
—Günther
[ 0.495214] Data abort info:
[ 0.495298] Access size = 8 byte(s)
[ 0.495408] SSE = 0, SRT = 0
[ 0.495519] SF = 1, AR = 1
[ 0.495636] CM = 0, WnR = 1
[ 0.495759] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000041645000
[ 0.495938] [ffff00000851036a] pgd=180000005fff8003, p4d=180000005fff8003, pud=180000005fff7003, pmd=180000005ffbd003, pte=0068000048510f07
[ 0.496779] Internal error: Oops: 0000000097c0c061 [#1] SMP
[ 0.497081] Modules linked in:
[ 0.497341] CPU: 0 PID: 9 Comm: kworker/u2:0 Not tainted 6.0.0-08005-g5622ae16a601 #1
[ 0.497643] Hardware name: linux,dummy-virt (DT)
[ 0.497987] Workqueue: events_unbound async_run_entry_fn
[ 0.498635] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.498882] pc : apparmor_file_alloc_security+0x98/0x210
[ 0.499132] lr : apparmor_file_alloc_security+0x48/0x210
[ 0.499297] sp : ffff800008093960
[ 0.499403] x29: ffff800008093960 x28: ffff800008093b30 x27: ffff000002201500
[ 0.499679] x26: ffffa4c2a0e55de0 x25: ffff00000201cd05 x24: ffffa4c2a1cd0068
[ 0.499901] x23: ffff000008510362 x22: ffff000008510360 x21: 0000000000000002
[ 0.500153] x20: ffffa4c2a0f72000 x19: ffff00000201b2b0 x18: ffffffffffffffff
[ 0.500375] x17: 000000000000003f x16: ffffa4c2a15d5008 x15: 0000000000000000
[ 0.500606] x14: 0000000000000001 x13: 0000000000002578 x12: ffff00001fef1eb8
[ 0.500830] x11: ffffa4c2a15ec860 x10: 0000000000000007 x9 : ffffa4c2a0bce9ec
[ 0.501061] x8 : ffff000008510380 x7 : 0000000000000000 x6 : 0000000000001e23
[ 0.501284] x5 : ffff000008510360 x4 : ffff800008093990 x3 : ffff000002017d80
[ 0.501500] x2 : 0000000000000001 x1 : ffff00000851036a x0 : ffff00000201b2b0
[ 0.501800] Call trace:
[ 0.501957] apparmor_file_alloc_security+0x98/0x210
[ 0.502241] security_file_alloc+0x6c/0xf0
[ 0.502401] __alloc_file+0x5c/0x100
[ 0.502520] alloc_empty_file+0x68/0x110
[ 0.502630] path_openat+0x50/0x1090
[ 0.502743] do_filp_open+0x88/0x13c
[ 0.502858] filp_open+0x110/0x1b0
[ 0.502961] do_name+0xbc/0x230
[ 0.503105] write_buffer+0x40/0x60
[ 0.503234] unpack_to_rootfs+0x100/0x2bc
[ 0.503375] do_populate_rootfs+0x70/0x134
[ 0.503516] async_run_entry_fn+0x40/0x1e0
[ 0.503653] process_one_work+0x1f4/0x460
[ 0.503783] worker_thread+0x188/0x4e0
[ 0.503902] kthread+0xe0/0xe4
[ 0.503999] ret_from_fork+0x10/0x20
[ 0.504279] Code: 52800002 d2800000 d2800013 910022e1 (c89ffc20)
[ 0.504647] ---[ end trace 0000000000000000 ]---
...
I am not sure what else I can provide in order to reproduce this but I
am happy to do whatever is needed to get to the bottom of this.
Thank you Nathan! I am able to reproduce this now with the .config you
provided and will have a look.
—Günther
--
