[PATCH 0/3] automation: add livepatch testing

[PATCH 0/3] automation: add livepatch testing

[Roger Pau Monne]

Hello,

The following series attempts to introduce some basic livepatch testing
in the gitlab CI loop.  Such testing will be more similar to what
downstreams use, as the patch payload will be built using
livepatch-build-tools.

A sample gitlab run can be found at:

https://gitlab.com/xen-project/people/royger/xen/-/pipelines/1082636267

The series introduces a new build and test steps.

Thanks, Roger.

Roger Pau Monne (3):
  automation/alpine: add elfutils-dev and coreutils for
    livepatch-build-tools
  livepatch: add a dummy hypercall for testing purposes
  automation: add x86-64 livepatching test

 automation/build/alpine/3.18.dockerfile       |  4 +
 automation/gitlab-ci/build.yaml               |  8 ++
 automation/gitlab-ci/test.yaml                |  8 ++
 automation/scripts/build                      | 13 +++
 .../scripts/qemu-alpine-x86_64-livepatch.sh   | 79 +++++++++++++++++++
 tools/include/xenctrl.h                       |  3 +
 tools/libs/ctrl/xc_misc.c                     | 14 ++++
 tools/misc/xen-livepatch.c                    | 25 ++++++
 xen/common/Makefile                           |  2 +-
 xen/common/livepatch-test.c                   | 20 +++++
 xen/common/livepatch.c                        |  4 +
 xen/include/public/sysctl.h                   |  7 ++
 xen/include/xen/livepatch.h                   |  4 +
 xen/test/livepatch/patches/test1.patch        | 13 +++
 14 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-alpine-x86_64-livepatch.sh
 create mode 100644 xen/common/livepatch-test.c
 create mode 100644 xen/test/livepatch/patches/test1.patch

-- 
2.43.0

[PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools

[Roger Pau Monne]

In preparation for adding some livepatch-build-tools test update the Alpine
container to also install elfutils-dev, coreutils and GNU awk.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
I don't very much like to add coreutils and gawk, as it's also good to test
that we can build Xen with Busybox, but I also got tired of adjusting
livepatch-build-tools.
---
 automation/build/alpine/3.18.dockerfile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/automation/build/alpine/3.18.dockerfile b/automation/build/alpine/3.18.dockerfile
index 4ae9cb5e9e30..fa6789347d87 100644
--- a/automation/build/alpine/3.18.dockerfile
+++ b/automation/build/alpine/3.18.dockerfile
@@ -47,3 +47,7 @@ RUN apk --no-cache add \
   libcap-ng-dev \
   ninja \
   pixman-dev \
+  # livepatch-tools deps
+  elfutils-dev \
+  coreutils \
+  gawk \
-- 
2.43.0

[PATCH 2/3] livepatch: add a dummy hypercall for testing purposes

[Roger Pau Monne]

Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to
test livepatch functionality.  The hypercall fills a value in the passed
structure, which is returned to the caller.

The xen-livepatch utility is expanded to allow calling that hypercall, and
printing the returned value on stdout.

Finally, add dummy patch that changes the returned value of the hypercall from
1 to 2.  Such patch can be used with livepatch-build-tools in order to generate
a livepatch payload, that when applied to the hypervisor change the printed
value of `xen-livepatch test`.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
The whole logic is very simple now.  I think it's enough to have a skeleton we
can later expand.

Unsure whether we should do some kind of test (with `patch -F0`) that the patch
still applies cleanly as part of Xen build.
---
 tools/include/xenctrl.h                |  3 +++
 tools/libs/ctrl/xc_misc.c              | 14 ++++++++++++++
 tools/misc/xen-livepatch.c             | 25 +++++++++++++++++++++++++
 xen/common/Makefile                    |  2 +-
 xen/common/livepatch-test.c            | 20 ++++++++++++++++++++
 xen/common/livepatch.c                 |  4 ++++
 xen/include/public/sysctl.h            |  7 +++++++
 xen/include/xen/livepatch.h            |  4 ++++
 xen/test/livepatch/patches/test1.patch | 13 +++++++++++++
 9 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/livepatch-test.c
 create mode 100644 xen/test/livepatch/patches/test1.patch

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 2ef8b4e05422..83a00d4974dd 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2645,6 +2645,9 @@ int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint32_
 int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags);
 int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags);
 
+/* Dummy hypercall to test livepatch functionality. */
+int xc_livepatch_test(xc_interface *xch, uint32_t *result);
+
 /*
  * Ensure cache coherency after memory modifications. A call to this function
  * is only required on ARM as the x86 architecture provides cache coherency
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 5ecdfa2c7934..0ca86a53d097 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -1021,6 +1021,20 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32
     return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
 }
 
+int xc_livepatch_test(xc_interface *xch, uint32_t *result)
+{
+    struct xen_sysctl sysctl = {
+        .cmd = XEN_SYSCTL_livepatch_op,
+        .u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_TEST,
+    };
+    int rc = do_sysctl(xch, &sysctl);
+
+    if ( !rc )
+        *result = sysctl.u.livepatch.u.test.result;
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 5bf9d9a32b65..5f6fd20d8814 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -37,6 +37,7 @@ void show_help(void)
             "  replace <name>         apply <name> patch and revert all others.\n"
             "  unload <name>          unload name <name> patch.\n"
             "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
+            "  test                   print the result of the test hypercall (for testing purposes only)\n"
             "    Supported flags:\n"
             "      --nodeps           Disable inter-module buildid dependency check.\n"
             "                         Check only against hypervisor buildid.\n",
@@ -542,6 +543,29 @@ error:
     return rc;
 }
 
+static int test_func(int argc, char *argv[])
+{
+    int rc;
+    uint32_t result = 0;
+
+    if ( argc != 0 )
+    {
+        show_help();
+        return -1;
+    }
+
+    rc = xc_livepatch_test(xch, &result);
+    if ( rc )
+    {
+        fprintf(stderr, "test operation failed: %s\n", strerror(errno));
+        return -1;
+    }
+
+    printf("%u\n", result);
+
+    return 0;
+}
+
 /*
  * These are also functions in action_options that are called in case
  * none of the ones in main_options match.
@@ -554,6 +578,7 @@ struct {
     { "list", list_func },
     { "upload", upload_func },
     { "load", load_func },
+    { "test", test_func },
 };
 
 int main(int argc, char *argv[])
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 69d6aa626c7f..ab073d41f1d2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -23,7 +23,7 @@ obj-y += kernel.o
 obj-y += keyhandler.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
-obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o livepatch-test.o
 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
 obj-y += multicall.o
diff --git a/xen/common/livepatch-test.c b/xen/common/livepatch-test.c
new file mode 100644
index 000000000000..05b638b2ac67
--- /dev/null
+++ b/xen/common/livepatch-test.c
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Dummy file for testing livepatch functionality. */
+#include <xen/livepatch.h>
+
+int livepatch_test(struct xen_sysctl_livepatch_test *test)
+{
+    test->result = 1;
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 1209fea2566c..e8894db1cc93 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -2116,6 +2116,10 @@ int livepatch_op(struct xen_sysctl_livepatch_op *livepatch)
         rc = livepatch_action(&livepatch->u.action);
         break;
 
+    case XEN_SYSCTL_LIVEPATCH_TEST:
+        rc = livepatch_test(&livepatch->u.test);
+        break;
+
     default:
         rc = -EOPNOTSUPP;
         break;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9b19679caeb1..9c13a7fdb22c 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1137,6 +1137,12 @@ struct xen_sysctl_livepatch_action {
     uint32_t pad;                           /* IN: Always zero. */
 };
 
+/* Dummy hypercall for testing live patches. */
+#define XEN_SYSCTL_LIVEPATCH_TEST 4
+struct xen_sysctl_livepatch_test {
+    uint32_t result;                        /* OUT: dummy result for testing. */
+};
+
 struct xen_sysctl_livepatch_op {
     uint32_t cmd;                           /* IN: XEN_SYSCTL_LIVEPATCH_*. */
     uint32_t pad;                           /* IN: Always zero. */
@@ -1145,6 +1151,7 @@ struct xen_sysctl_livepatch_op {
         struct xen_sysctl_livepatch_list list;
         struct xen_sysctl_livepatch_get get;
         struct xen_sysctl_livepatch_action action;
+        struct xen_sysctl_livepatch_test test;
     } u;
 };
 
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index df339a134e40..60d11d037dfb 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -11,6 +11,8 @@ struct livepatch_elf_sec;
 struct livepatch_elf_sym;
 struct xen_sysctl_livepatch_op;
 
+#include <xen/types.h> /* For elfstructs.h */
+
 #include <xen/elfstructs.h>
 #include <xen/errno.h> /* For -ENOSYS or -EOVERFLOW */
 
@@ -165,6 +167,8 @@ static inline void common_livepatch_revert(const struct livepatch_func *func,
     arch_livepatch_revert(func, state);
     state->applied = LIVEPATCH_FUNC_NOT_APPLIED;
 }
+
+int livepatch_test(struct xen_sysctl_livepatch_test *test);
 #else
 
 /*
diff --git a/xen/test/livepatch/patches/test1.patch b/xen/test/livepatch/patches/test1.patch
new file mode 100644
index 000000000000..c07d697cc8de
--- /dev/null
+++ b/xen/test/livepatch/patches/test1.patch
@@ -0,0 +1,13 @@
+diff --git a/xen/common/livepatch-test.c b/xen/common/livepatch-test.c
+index 05b638b2ac..876173ab6f 100644
+--- a/xen/common/livepatch-test.c
++++ b/xen/common/livepatch-test.c
+@@ -5,7 +5,7 @@
+ 
+ int livepatch_test(struct xen_sysctl_livepatch_test *test)
+ {
+-    test->result = 1;
++    test->result = 2;
+     return 0;
+ }
+ 
-- 
2.43.0

[PATCH 3/3] automation: add x86-64 livepatching test

[Roger Pau Monne]

Introduce a new gitlab tests for livepatching, using livepatch-build-tools,
which better reflects how downstreams build live patches rather than the
in-tree tests.

The tests applies the dummy in-tree patch example, checks that the patch is
applied correctly and then reverts and unloads it.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 automation/gitlab-ci/build.yaml               |  8 ++
 automation/gitlab-ci/test.yaml                |  8 ++
 automation/scripts/build                      | 13 +++
 .../scripts/qemu-alpine-x86_64-livepatch.sh   | 79 +++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100755 automation/scripts/qemu-alpine-x86_64-livepatch.sh

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 32af30ccedc9..22026df51b87 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -358,6 +358,14 @@ alpine-3.18-gcc-debug:
   variables:
     CONTAINER: alpine:3.18
 
+alpine-3.18-gcc-livepatch:
+  extends: .gcc-x86-64-build
+  variables:
+    CONTAINER: alpine:3.18
+    LIVEPATCH: y
+    EXTRA_XEN_CONFIG: |
+      CONFIG_LIVEPATCH=y
+
 debian-stretch-gcc-debug:
   extends: .gcc-x86-64-build-debug
   variables:
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d156f..58a90be5ed0e 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -459,3 +459,11 @@ qemu-smoke-ppc64le-powernv9-gcc:
   needs:
     - qemu-system-ppc64-8.1.0-ppc64-export
     - debian-bullseye-gcc-ppc64le-debug
+
+qemu-alpine-x86_64-gcc-livepatch:
+  extends: .qemu-x86-64
+  script:
+    - ./automation/scripts/qemu-alpine-x86_64-livepatch.sh 2>&1 | tee ${LOGFILE}
+  needs:
+    - *x86-64-test-needs
+    - alpine-3.18-gcc-livepatch
diff --git a/automation/scripts/build b/automation/scripts/build
index b3c71fb6fb60..7ae735fc193e 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -103,3 +103,16 @@ else
     cp -r dist binaries/
     if [[ -f xen/xen ]] ; then cp xen/xen binaries/xen; fi
 fi
+
+if [[ "$LIVEPATCH" == "y" ]]; then
+    # Build a test livepatch using livepatch-build-tools.
+
+    BUILDID=$(readelf -Wn xen/xen-syms | sed -n -e 's/^.*Build ID: //p')
+
+    git clone https://xenbits.xen.org/git-http/livepatch-build-tools.git
+    cd livepatch-build-tools
+    make
+    ./livepatch-build -s ../ -p ../xen/test/livepatch/patches/test1.patch \
+        -o out -c ../xen/.config --depends $BUILDID --xen-depends $BUILDID
+    cp out/test1.livepatch ../binaries/test1.livepatch
+fi
diff --git a/automation/scripts/qemu-alpine-x86_64-livepatch.sh b/automation/scripts/qemu-alpine-x86_64-livepatch.sh
new file mode 100755
index 000000000000..9b27a01b07f0
--- /dev/null
+++ b/automation/scripts/qemu-alpine-x86_64-livepatch.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+
+set -ex
+
+cd binaries
+# initrd.tar.gz is Dom0 rootfs
+mkdir -p rootfs
+cd rootfs
+tar xvzf ../initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../dist/install/* .
+cp ../test1.livepatch ./root/
+cat << "EOF" >> etc/local.d/xen.start
+#!/bin/bash
+
+set -ex
+
+trap poweroff EXIT
+
+export LD_LIBRARY_PATH=/usr/local/lib
+
+result=`xen-livepatch test`
+if [ "$result" != "1" ]; then
+    echo "FAIL"
+    exit 1
+fi
+
+xen-livepatch load /root/test1.livepatch
+
+result=`xen-livepatch test`
+if [ "$result" != "2" ]; then
+    echo "FAIL"
+    exit 1
+fi
+
+xen-livepatch revert test1
+xen-livepatch unload test1
+
+result=`xen-livepatch test`
+if [ "$result" != "1" ]; then
+    echo "FAIL"
+    exit 1
+fi
+
+echo "SUCCESS"
+EOF
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+# rebuild Dom0 rootfs
+find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
+cd ../..
+
+cat >> binaries/pxelinux.0 << EOF
+#!ipxe
+
+kernel xen console=com1 console_timestamps=boot
+module bzImage console=hvc0
+module xen-rootfs.cpio.gz
+boot
+EOF
+
+# Run the test
+rm -f smoke.serial
+timeout -k 1 360 \
+qemu-system-x86_64 \
+    -cpu qemu64,+svm \
+    -m 2G -smp 2 \
+    -monitor none -serial stdio \
+    -nographic \
+    -device virtio-net-pci,netdev=n0 \
+    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0 |& \
+        tee smoke.serial | sed 's/\r//'
+
+grep -q "SUCCESS" smoke.serial
+exit 0
-- 
2.43.0

Re: [PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools

[Andrew Cooper]

On 23/11/2023 11:23 am, Roger Pau Monne wrote:
> In preparation for adding some livepatch-build-tools test update the Alpine
> container to also install elfutils-dev, coreutils and GNU awk.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> I don't very much like to add coreutils and gawk, as it's also good to test
> that we can build Xen with Busybox, but I also got tired of adjusting
> livepatch-build-tools.

Right now, the alpine environment is the main one which spots violations
of Xen's requirement for simply a POSIX-compliant awk, which I think
this would break?

How much effort would it be fix livepatch-build-tools?  I think that
would be preferable, and could be persuaded to do some simple busywork...

What about coreutils?  Presumably that's down to some differences from
busybox ?

~Andrew

Re: [PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools

[Roger Pau Monné]

On Thu, Nov 23, 2023 at 11:51:33AM +0000, Andrew Cooper wrote:
> On 23/11/2023 11:23 am, Roger Pau Monne wrote:
> > In preparation for adding some livepatch-build-tools test update the Alpine
> > container to also install elfutils-dev, coreutils and GNU awk.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > I don't very much like to add coreutils and gawk, as it's also good to test
> > that we can build Xen with Busybox, but I also got tired of adjusting
> > livepatch-build-tools.
> 
> Right now, the alpine environment is the main one which spots violations
> of Xen's requirement for simply a POSIX-compliant awk, which I think
> this would break?

Likely, we also test on Cirrus using FreeBSD, so we would at least
spot instances where the extensions are not implemented by BSD sed.

> How much effort would it be fix livepatch-build-tools?  I think that
> would be preferable, and could be persuaded to do some simple busywork...

I can give it a try.

> What about coreutils?  Presumably that's down to some differences from
> busybox ?

Yeah, it's for the usage of `readlink -m` by livepatch-build-tools.

We will have to switch to using `readlink -f`, as that's the only
canonicalize option supported by BusyBox readlink.  Note that anyway
`readlink` is not part of POSIX.

Thanks, Roger.

Re: [PATCH 2/3] livepatch: add a dummy hypercall for testing purposes

[Andrew Cooper]

On 23/11/2023 11:23 am, Roger Pau Monne wrote:
> Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to
> test livepatch functionality.  The hypercall fills a value in the passed
> structure, which is returned to the caller.
>
> The xen-livepatch utility is expanded to allow calling that hypercall, and
> printing the returned value on stdout.
>
> Finally, add dummy patch that changes the returned value of the hypercall from
> 1 to 2.  Such patch can be used with livepatch-build-tools in order to generate
> a livepatch payload, that when applied to the hypervisor change the printed
> value of `xen-livepatch test`.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> The whole logic is very simple now.  I think it's enough to have a skeleton we
> can later expand.
>
> Unsure whether we should do some kind of test (with `patch -F0`) that the patch
> still applies cleanly as part of Xen build.

Thanks for looking into this.  I think one tweak towards the larger plan
might help here.

When I envisaged this originally, it was something along the lines of
test_self_modify_code() which would be called on boot after alternatives
were called, which would sanity check the result of certain simple cases.

Then, for livepatching, I was thinking of something like this:

obj-y += test_smc.o
targets-y += test_smc_alt.o

i.e. we have test_smc.c and test_smc_alt.c which are two slightly
different copies of the same thing, and we compile both but don't link
the second one in.

Then, we can diff the two C files in order to make the patch to build as
a livepatch.  This way we're not maintaining a patch committed into the
tree, which I suspect will make everyone's lives easier.  I suspect in
practice that we'll want test_smc_asm.S pairs too.

Not necessarily for now, but I was also thinking we'd have a test stage
where we know exactly what the livepatch ought to look like, so we audit
the eventual livepatch elf that it has all the expected differences
encoded, and no extraneous ones.

Finally, I was thinking that the hypercall would (re)run
test_self_modify_code().  I'm on the fence about making it part of the
livepatch infrastructure, given that the nature of the test is wider,
but I can't think of any case that we'd be wanting runtime self
modifying code (e.g. rerun alternatives after ucode load) which isn't
linked to a new livepatch to begin with.

Thoughts?

~Andrew