lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+CK2bAWe15SkcvWx_hRHvT-RAcudKQ1hRV1htuWanh9Mbh_YA@mail.gmail.com>
Date: Tue, 25 Nov 2025 13:42:25 -0500
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Mike Rapoport <rppt@...nel.org>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com, 
	dmatlack@...gle.com, rientjes@...gle.com, corbet@....net, 
	rdunlap@...radead.org, ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com, 
	ojeda@...nel.org, aliceryhl@...gle.com, masahiroy@...nel.org, 
	akpm@...ux-foundation.org, tj@...nel.org, yoann.congal@...le.fr, 
	mmaurer@...gle.com, roman.gushchin@...ux.dev, chenridong@...wei.com, 
	axboe@...nel.dk, mark.rutland@....com, jannh@...gle.com, 
	vincent.guittot@...aro.org, hannes@...xchg.org, dan.j.williams@...el.com, 
	david@...hat.com, joel.granados@...nel.org, rostedt@...dmis.org, 
	anna.schumaker@...cle.com, song@...nel.org, linux@...ssschuh.net, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-mm@...ck.org, 
	gregkh@...uxfoundation.org, tglx@...utronix.de, mingo@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org, 
	cw00.choi@...sung.com, myungjoo.ham@...sung.com, yesanishhere@...il.com, 
	Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com, 
	aleksander.lobakin@...el.com, ira.weiny@...el.com, 
	andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de, 
	bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com, 
	stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net, 
	brauner@...nel.org, linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	saeedm@...dia.com, ajayachandra@...dia.com, jgg@...dia.com, parav@...dia.com, 
	leonro@...dia.com, witu@...dia.com, hughd@...gle.com, skhawaja@...gle.com, 
	chrisl@...nel.org
Subject: Re: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure
 and scripts

On Mon, Nov 24, 2025 at 2:54 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> On Sat, Nov 22, 2025 at 05:23:46PM -0500, Pasha Tatashin wrote:
> > Subject: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure and scripts
>
> Maybe                                                ^ end to end

Done.

>
> > Add the testing infrastructure required to verify the liveupdate
> > feature. This includes a custom init process, a test orchestration
> > script, and a batch runner.
>
> And say here that it's end to end test.

Done

> > +static int is_stage_2(void)
> > +{
> > +     char cmdline[COMMAND_LINE_SIZE];
> > +     ssize_t len;
> > +     int fd;
> > +
> > +     fd = open("/proc/cmdline", O_RDONLY);
> > +     if (fd < 0)
> > +             return 0;
> > +
> > +     len = read(fd, cmdline, sizeof(cmdline) - 1);
> > +     close(fd);
> > +
> > +     if (len < 0)
> > +             return 0;
>
> Shouldn't we bail out of the test if read of command line failed?

Sure, done.

> > +function cleanup() {
> > +     local exit_code=$?
> > +
> > +     if [ -z "$workspace_dir" ]; then
> > +             ktap_finished
> > +             return
> > +     fi
> > +
> > +     if [ $exit_code -ne 0 ]; then
> > +             echo "# Test failed (exit code $exit_code)."
> > +             echo "# Workspace preserved at: $workspace_dir"
> > +     elif [ "$KEEP_WORKSPACE" -eq 1 ]; then
> > +             echo "# Workspace preserved (user request) at: $workspace_dir"
> > +     else
> > +             rm -fr "$workspace_dir"
> > +     fi
> > +     ktap_finished
>
>         exit $exit_code

Done

> > +function build_kernel() {
> > +     local build_dir=$1
> > +     local make_cmd=$2
> > +     local kimage=$3
> > +     local target_arch=$4
> > +
> > +     local kconfig="$build_dir/.config"
> > +     local common_conf="$test_dir/config"
> > +     local arch_conf="$test_dir/config.$target_arch"
> > +
> > +     echo "# Building kernel in: $build_dir"
> > +     $make_cmd defconfig
> > +
> > +     local fragments=""
> > +     if [[ -f "$common_conf" ]]; then
> > +             fragments="$fragments $common_conf"
> > +     fi
>
> Without this CONFIG_LIVEUPDATE won't be set
> > +
> > +     if [[ -f "$arch_conf" ]]; then
> > +             fragments="$fragments $arch_conf"
> > +     fi
> > +
> > +     if [[ -n "$fragments" ]]; then
> > +             "$kernel_dir/scripts/kconfig/merge_config.sh" \
> > +                     -Q -m -O "$build_dir" "$kconfig" $fragments >> /dev/null
> > +     fi
>
> I believe you can just
>
>         cat $common_conf $fragments >  $build_dir/.config
>         make olddefconfig
>
> without running defconfig at the beginning
> It will build faster, just make sure to add CONFIG_SERIAL_ to $arch_conf

I will look into that, so how performance really changes,  I liked
using merge_config.sh as it does not print warnings.

>
> > +     $make_cmd olddefconfig
> > +     $make_cmd "$kimage"
> > +     $make_cmd headers_install INSTALL_HDR_PATH="$headers_dir"
> > +}
> > +
> > +function mkinitrd() {
> > +     local build_dir=$1
> > +     local kernel_path=$2
> > +     local test_name=$3
> > +
> > +     # 1. Compile the test binary and the init process
>
> Didn't find 2. ;-)
> Don't think we want the numbering here, plain comments are fine

Updated comment.


>
> > +     "$CROSS_COMPILE"gcc -static -O2 \
> > +             -I "$headers_dir/include" \
> > +             -I "$test_dir" \
> > +             -o "$workspace_dir/test_binary" \
> > +             "$test_dir/$test_name.c" "$test_dir/luo_test_utils.c"
>
> This will have hard time cross-compiling with -nolibc toolchains

Hm, it works for me, I am not sure with nolibc cross compiler, am I
missing something?

>
> > +
> > +     "$CROSS_COMPILE"gcc -s -static -Os -nostdinc -nostdlib          \
> > +                     -fno-asynchronous-unwind-tables -fno-ident      \
> > +                     -fno-stack-protector                            \
> > +                     -I "$headers_dir/include"                       \
> > +                     -I "$kernel_dir/tools/include/nolibc"           \
> > +                     -o "$workspace_dir/init" "$test_dir/init.c"
>
> This failed for me with gcc 14.2.0 (Debian 14.2.0-19):


Updated, removed the extra const, and static.

>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c: In function ‘run_test’:
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: error: initializer element is not constant
>   111 |             static const char *const argv[] = {TEST_BINARY, stage_arg, NULL};
>       |                                                             ^~~~~~~~~
>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: note: (near initialization for ‘argv[1]’)
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:113:37: error: passing argument 2 of ‘execve’ from incompatible pointer type [-Wincompatible-pointer-types]
>   113 |                 execve(TEST_BINARY, argv, NULL);
>       |                                     ^~~~
>       |                                     |
>       |                                     const char * const*
> In file included from /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:16:
> /usr/include/unistd.h:572:52: note: expected ‘char * const*’ but argument is of type ‘const char * const*’
>   572 | extern int execve (const char *__path, char *const __argv[],
>       |                                        ~~~~~~~~~~~~^~~~~~~~
>
> > +
> > +     cat > "$workspace_dir/cpio_list_inner" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> Don't you need /dev/liveupdate node?

That should be created by the kernel itself.

>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +EOF
> > +
> > +     # Generate inner_initrd.cpio
> > +     "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list_inner" > "$workspace_dir/inner_initrd.cpio"
> > +
> > +     cat > "$workspace_dir/cpio_list" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> And here as well.

Not needed.

>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /kernel $kernel_path 0644 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +file /initrd.img $workspace_dir/inner_initrd.cpio 0644 0 0
> > +EOF
> > +
> > +     # Generate the final initrd
> > +     "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list" > "$initrd"
> > +     local size=$(du -h "$initrd" | cut -f1)
> > +}
> > +
> > +function run_qemu() {
> > +     local qemu_cmd=$1
> > +     local cmdline=$2
> > +     local kernel_path=$3
> > +     local serial="$workspace_dir/qemu.serial"
> > +
> > +     local accel="-accel tcg"
> > +     local host_machine=$(uname -m)
> > +
> > +     [[ "$host_machine" == "arm64" ]] && host_machine="aarch64"
> > +     [[ "$host_machine" == "x86_64" ]] && host_machine="x86_64"
> > +
> > +     if [[ "$qemu_cmd" == *"$host_machine"* ]]; then
> > +             if [ -w /dev/kvm ]; then
> > +                     accel="-accel kvm"
>
> Just pass both kvm and tcg and let qemu complain.

I hated those warnings, this is why I added this "if" in the first place :-)

Thank you for your reviews, I am going to send this patch separately
from this series, so let's continue the discussion there.

Pasha

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ