Fix: bytecode validator: oops during validation of immediate string
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 5 Oct 2023 21:02:57 +0000 (17:02 -0400)
committerMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fri, 6 Oct 2023 14:59:30 +0000 (10:59 -0400)
commita62977cac65f63f20625dd501282de1ea0156ee0
tree191278056e46913f8c00d06204db0c5dfbee9ec7
parent6be48c9f6a32cdcab9824aaad0c19f92222633c4
Fix: bytecode validator: oops during validation of immediate string

Issue observed
--------------

Running Linux 6.5.5, lttng-modules @ 6be48c9f, all built with gcc
13.2.1, I got a 'BUG' in dmesg while enabling the following event
rule:

  $ lttng enable-event --kernel --syscall --channel chanK --all --filter '$ctx.procname == "UST reg*"'

The relevant parts of the 'BUG' output follow:

  [  +0.715480] detected buffer overflow in strnlen
  [  +0.000001] kernel BUG at lib/string_helpers.c:1031!
  [  +0.000008] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
  [  +0.000003] CPU: 2 PID: 157174 Comm: Client manageme Tainted: G S   U     OE      6.5.5-arch1-1 #1 d82a0f532dd8cfe67d5795c1738d9c01059a0c62
  [  +0.000001] RIP: 0010:fortify_panic+0x13/0x20
  [  +0.000006] Code: 41 5d c3 cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 48 89 fe 48 c7 c7 90 22 c8 86 e8 3d aa b1 ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90
  [  +0.000002] RSP: 0018:ffffa7c7c106f918 EFLAGS: 00010246
  [  +0.000002] RAX: 0000000000000023 RBX: 000000000000000b RCX: 0000000000000000
  [  +0.000002] RDX: 0000000000000000 RSI: ffff92766e4a16c0 RDI: ffff92766e4a16c0
  [  +0.000001] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffa7c7c106f7c0
  [  +0.000001] R10: 0000000000000003 R11: ffffffff874ca068 R12: ffff927618202480
  [  +0.000001] R13: ffff9276182024d2 R14: ffff927453999c08 R15: ffff9273dc7aa478
  [  +0.000001] FS:  00007f06553f9680(0000) GS:ffff92766e480000(0000) knlGS:0000000000000000
  [  +0.000002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [  +0.000002] CR2: 0000556d54eceaa8 CR3: 00000001ad9de002 CR4: 00000000003706e0
  [  +0.000001] Call Trace:
  [  +0.000002]  <TASK>
  [  +0.000002]  ? die+0x36/0x90
  [  +0.000004]  ? do_trap+0xda/0x100
  [  +0.000003]  ? fortify_panic+0x13/0x20
  [  +0.000002]  ? do_error_trap+0x6a/0x90
  [  +0.000002]  ? fortify_panic+0x13/0x20
  [  +0.000002]  ? exc_invalid_op+0x50/0x70
  [  +0.000003]  ? fortify_panic+0x13/0x20
  [  +0.000002]  ? asm_exc_invalid_op+0x1a/0x20
  [  +0.000005]  ? fortify_panic+0x13/0x20
  [  +0.000002]  ? fortify_panic+0x13/0x20
  [  +0.000003]  bytecode_validate_overflow+0x155/0x1f0 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000330]  lttng_bytecode_validate_load+0x32/0x1e0 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000183]  lttng_enabler_link_bytecode+0x135/0x5a0 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000132]  lttng_sync_event_list+0xef/0x650 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000123]  ? __wake_up_common+0x73/0x180
  [  +0.000004]  lttng_session_enable+0x3e/0x130 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000121]  lttng_session_ioctl+0x5db/0x720 [lttng_tracer 759e3e4fee0e774ef575e93b67e8dc7955d0c2c2]
  [  +0.000120]  ? __slab_free+0xf1/0x330
  [  +0.000004]  ? __scm_recv_common.isra.0+0x144/0x180
  [  +0.000004]  ? unix_stream_read_generic+0x233/0xb60
  [  +0.000006]  __x64_sys_ioctl+0x94/0xd0
  [  +0.000004]  do_syscall_64+0x5d/0x90
  [  +0.000004]  ? switch_fpu_return+0x50/0xe0
  [  +0.000004]  ? exit_to_user_mode_prepare+0x132/0x1e0
  [  +0.000003]  ? syscall_exit_to_user_mode+0x2b/0x40
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000003]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? syscall_exit_to_user_mode+0x2b/0x40
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? do_syscall_64+0x6c/0x90
  [  +0.000002]  ? exc_page_fault+0x7f/0x180
  [  +0.000003]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Cause
-----

`struct load_op` has a trailing 0-length array `data` member that is
used to refer, in the context of BYTECODE_OP_LOAD_STAR_GLOB_STRING, to
an immediate string operand that follows it.

During the validation of a filtering bytecode, strnlen is properly used
to determine the size of the immediate string operand, with a `maxlen`
parameter that is used to ensure the string operand is contained within
the bytecode (see lttng-bytecode-validator.c:434).

However, recent KSPP-related changes have enabled additional overrun
checks when statically-sized and flexible arrays are used. Those are
enabled when the kernel is built with CONFIG_UBSAN_BOUNDS and/or
CONFIG_FORTIFY_SOURCE configured.

The KBUILD CFLAGS now contain `-fstrict-flex-arrays=3`, which is
recognized by gcc 13+[1] and allows proper coverage of dynamically sized
trailing arrays when those configuration options are used.

With those validations in place, the kernel assumes that the `data`
array is truly of length 0 and it BUGs to warn of an invalid access.

The commit linked above contains a number of links explaining the
rationale for transitioning uses of the trailing zero-length arrays (a
gcc extension) to C99 flexible array members (FAM).

This was discussed at this year's GNU Cauldron [2].

Solution
--------

Uses of zero-length arrays (`foo[0]`) are replaced by flexible array
members (`foo[]`). The only cases that are left untouched are those
where the zero-length array is used to indicate the end of a
structure (i.e. it doesn't indicate that a variable number of elements
follow), see the `metadata_packet_header`, `metadata_record_header`,
`event_notifier_packet_header`, and `event_notifier_record_header`
structures.

It may be desirable to use the new `counted_by` attribute for some of
those in the future (`lttng_kernel_abi_filter_bytecode`,
`lttng_kernel_abi_capture_bytecode`, and `bytecode_runtime`) [3].

Note
----

While this is tagged as a memory handling 'fix', it has no security
implication as far as I can tell. The accesses that are flagged by the
new validations were valid.

This merely allows the runtime validations to understand the memory
layout properly.

[1] https://github.com/torvalds/linux/commit/df8fc4e934c12b906d08050d7779f292b9c5c6b5
[2] https://gcc.gnu.org/wiki/cauldron2023talks?action=AttachFile&do=get&target=Most-wanted+Security+Features+in+GCC+for+Linux+Kernel.pdf
[3] https://lwn.net/Articles/930943/

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Id39b101aaafe68f8fae6b86cd61806cba8cb1e6a
include/lttng/abi.h
include/lttng/bytecode.h
include/lttng/lttng-bytecode.h
src/lttng-tracepoint.c
This page took 0.027914 seconds and 4 git commands to generate.