[v2] classes/rootfs: remove content of /dev, /run and vmlinux old

Message ID 20240425071419.292013-1-Quirin.Gylstorff@siemens.com
State Under Review
Headers show
Series [v2] classes/rootfs: remove content of /dev, /run and vmlinux old | expand

Commit Message

Quirin Gylstorff April 25, 2024, 7:13 a.m. UTC
From: Quirin Gylstorff <quirin.gylstorff@siemens.com>

`/dev` and `/run` contain artifacts from the ISAR build clean them
up for reproducability.

Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
---
Changes v2:
 - move deletion to do_rootfs_finalize this avoids some race conditions
   during build
 - remove content of  /run
 - remove symblic link /vmlinuz.old
 meta/classes/image.bbclass | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Baurzhan Ismagulov April 26, 2024, 8:55 a.m. UTC | #1
On 2024-04-25 09:13, 'Quirin Gylstorff' via isar-users wrote:
> `/dev` and `/run` contain artifacts from the ISAR build clean them
> up for reproducability.

Suggest "Isar" and "reproducibility".


> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 98741da0..4af0d448 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -427,12 +427,17 @@ do_rootfs_finalize() {
>              rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
>  
>          mountpoint -q '${ROOTFSDIR}/dev' && \
> -            umount -l ${ROOTFSDIR}/dev
> +            umount -l -R ${ROOTFSDIR}/dev && \
> +            rm -rf ${ROOTFSDIR}/dev/*
> +
>          mountpoint -q '${ROOTFSDIR}/proc' && \
>              umount -l ${ROOTFSDIR}/proc
>          mountpoint -q '${ROOTFSDIR}/sys' && \
>              umount -l ${ROOTFSDIR}/sys
>  
> +        rm -rf ${ROOTFSDIR}/run/*
> +        rm -f ${ROOTFSDIR}/vmlinuz.old
> +

Could you provide the list of the specific files that you want to have removed?

Debian normally has /dev populated even though devtmpfs is mounted later on top
of it. If the problem is that the entries have different timestamps on
different builds, then the question is not limited to /dev but applies to any
files created in postinstall scripts. I don't think the general approach should
be to remove such files at the meta level.

On an unrelated note, we've tested non-lazy mounts and haven't observed any
issues; we'll share the patches for it as well as for recursive mounts.

With kind regards,
Baurzhan
MOESSBAUER, Felix April 26, 2024, 12:13 p.m. UTC | #2
On Fri, 2024-04-26 at 10:55 +0200, Baurzhan Ismagulov wrote:
> On 2024-04-25 09:13, 'Quirin Gylstorff' via isar-users wrote:
> > `/dev` and `/run` contain artifacts from the ISAR build clean them
> > up for reproducability.
> 
> Suggest "Isar" and "reproducibility".
> 
> 
> > diff --git a/meta/classes/image.bbclass
> > b/meta/classes/image.bbclass
> > index 98741da0..4af0d448 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -427,12 +427,17 @@ do_rootfs_finalize() {
> >              rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> >  
> >          mountpoint -q '${ROOTFSDIR}/dev' && \
> > -            umount -l ${ROOTFSDIR}/dev
> > +            umount -l -R ${ROOTFSDIR}/dev && \
> > +            rm -rf ${ROOTFSDIR}/dev/*
> > +
> >          mountpoint -q '${ROOTFSDIR}/proc' && \
> >              umount -l ${ROOTFSDIR}/proc
> >          mountpoint -q '${ROOTFSDIR}/sys' && \
> >              umount -l ${ROOTFSDIR}/sys
> >  
> > +        rm -rf ${ROOTFSDIR}/run/*
> > +        rm -f ${ROOTFSDIR}/vmlinuz.old
> > +
> 
> Could you provide the list of the specific files that you want to
> have removed?
> 
> Debian normally has /dev populated even though devtmpfs is mounted
> later on top

This is probably just an artifact of the bootstrapping, but not written
in any debian policy. As debian uses systemd (and we anyways only
support this init system), we should follow the "man file-hierarchy".
There a devtmpfs is always mounted on /dev. What's the point then to
still keep the dirs and files there?

> of it. If the problem is that the entries have different timestamps
> on
> different builds, then the question is not limited to /dev but
> applies to any
> files created in postinstall scripts. I don't think the general
> approach should
> be to remove such files at the meta level.

The timestamps are already clamped by the rootfs postprocessing,
however this is a leak of host resources into the rootfs. Which files
are under /dev depends on the devtmpfs implementation - which depends
on the host kernel.

I propose to just clear these directories.

> 
> On an unrelated note, we've tested non-lazy mounts and haven't
> observed any
> issues; we'll share the patches for it as well as for recursive
> mounts.

That would be great.

Felix

> 
> With kind regards,
> Baurzhan
Baurzhan Ismagulov May 2, 2024, 3:16 p.m. UTC | #3
On 2024-04-26 12:13, MOESSBAUER, Felix wrote:
> On Fri, 2024-04-26 at 10:55 +0200, Baurzhan Ismagulov wrote:
> > On 2024-04-25 09:13, 'Quirin Gylstorff' via isar-users wrote:
> > > `/dev` and `/run` contain artifacts from the ISAR build clean them
> > > up for reproducability.
> > 
> > Suggest "Isar" and "reproducibility".
> > 
> > 
> > > diff --git a/meta/classes/image.bbclass
> > > b/meta/classes/image.bbclass
> > > index 98741da0..4af0d448 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -427,12 +427,17 @@ do_rootfs_finalize() {
> > >              rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> > >  
> > >          mountpoint -q '${ROOTFSDIR}/dev' && \
> > > -            umount -l ${ROOTFSDIR}/dev
> > > +            umount -l -R ${ROOTFSDIR}/dev && \
> > > +            rm -rf ${ROOTFSDIR}/dev/*
> > > +
> > >          mountpoint -q '${ROOTFSDIR}/proc' && \
> > >              umount -l ${ROOTFSDIR}/proc
> > >          mountpoint -q '${ROOTFSDIR}/sys' && \
> > >              umount -l ${ROOTFSDIR}/sys
> > >  
> > > +        rm -rf ${ROOTFSDIR}/run/*
> > > +        rm -f ${ROOTFSDIR}/vmlinuz.old
> > > +
> > 
> > Could you provide the list of the specific files that you want to
> > have removed?
> > 
> > Debian normally has /dev populated even though devtmpfs is mounted
> > later on top
> 
> This is probably just an artifact of the bootstrapping, but not written
> in any debian policy. As debian uses systemd (and we anyways only
> support this init system), we should follow the "man file-hierarchy".
> There a devtmpfs is always mounted on /dev. What's the point then to
> still keep the dirs and files there?
> 
> > of it. If the problem is that the entries have different timestamps
> > on
> > different builds, then the question is not limited to /dev but
> > applies to any
> > files created in postinstall scripts. I don't think the general
> > approach should
> > be to remove such files at the meta level.
> 
> The timestamps are already clamped by the rootfs postprocessing,
> however this is a leak of host resources into the rootfs. Which files
> are under /dev depends on the devtmpfs implementation - which depends
> on the host kernel.
> 
> I propose to just clear these directories.

Thanks Felix for the details,

1. Regarding /dev, I agree that it looks like an artifact of bootstrapping and
   installation. However, I don't see that the contents are copied from
   devtmpfs; could you please provide a sample command line and the list of
   files to confirm the host leak?

   After both debootstrap and d-i, I see the fixed list of console, fd, full,
   null, ptmx, pts, random, shm, stderr, stdin, stdout, tty, urandom, zero. I
   do think we can safely remove /dev/* for our use cases (although
   file-hierarchy(7) says devtmpfs is only *usually* mounted), but I'd like to
   document the reason for removal if we later find out that some software has
   different expectations.

   We see different tools expecting at least /dev/null, /proc, /dev/pts and
   /dev/shm and I also couldn't find any documentation on this. In the good
   case, they fail (sometimes with cryptic messages), but often they crash or
   hog the CPU (which is fun to debug from under e.g. CI + qemu). Some
   indirectly related examples and discussion:
   https://salsa.debian.org/installer-team/debootstrap/-/commit/a10e577675896b693c66ac77eace7ff76199da2c
   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=571136

2. For /run, I'd still like to see the list of the files, because I'd expect
   Isar to leave it in a clean (= empty) state; maybe we have something that we
   should clean up elsewhere in Isar.

3. Regarding "vmlinux old", I assume it's about /vmlinuz.old (and I suggest to
   update the commit title for better understanding -- or we can do that when
   applying if it's ok for you) -- that should disappear by itself when we
   multistrap the final rootfs directly from multiple apt repos. I'm fine
   applying this part.

With kind regards,
Baurzhan
MOESSBAUER, Felix May 10, 2024, 3:29 p.m. UTC | #4
On Thu, 2024-05-02 at 17:16 +0200, Baurzhan Ismagulov wrote:
> On 2024-04-26 12:13, MOESSBAUER, Felix wrote:
> > On Fri, 2024-04-26 at 10:55 +0200, Baurzhan Ismagulov wrote:
> > > On 2024-04-25 09:13, 'Quirin Gylstorff' via isar-users wrote:
> > > > `/dev` and `/run` contain artifacts from the ISAR build clean
> > > > them
> > > > up for reproducability.
> > > 
> > > Suggest "Isar" and "reproducibility".
> > > 
> > > 
> > > > diff --git a/meta/classes/image.bbclass
> > > > b/meta/classes/image.bbclass
> > > > index 98741da0..4af0d448 100644
> > > > --- a/meta/classes/image.bbclass
> > > > +++ b/meta/classes/image.bbclass
> > > > @@ -427,12 +427,17 @@ do_rootfs_finalize() {
> > > >              rmdir --ignore-fail-on-non-empty
> > > > ${ROOTFSDIR}/base-apt
> > > >  
> > > >          mountpoint -q '${ROOTFSDIR}/dev' && \
> > > > -            umount -l ${ROOTFSDIR}/dev
> > > > +            umount -l -R ${ROOTFSDIR}/dev && \
> > > > +            rm -rf ${ROOTFSDIR}/dev/*
> > > > +
> > > >          mountpoint -q '${ROOTFSDIR}/proc' && \
> > > >              umount -l ${ROOTFSDIR}/proc
> > > >          mountpoint -q '${ROOTFSDIR}/sys' && \
> > > >              umount -l ${ROOTFSDIR}/sys
> > > >  
> > > > +        rm -rf ${ROOTFSDIR}/run/*
> > > > +        rm -f ${ROOTFSDIR}/vmlinuz.old
> > > > +
> > > 
> > > Could you provide the list of the specific files that you want to
> > > have removed?
> > > 
> > > Debian normally has /dev populated even though devtmpfs is
> > > mounted
> > > later on top
> > 
> > This is probably just an artifact of the bootstrapping, but not
> > written
> > in any debian policy. As debian uses systemd (and we anyways only
> > support this init system), we should follow the "man file-
> > hierarchy".
> > There a devtmpfs is always mounted on /dev. What's the point then
> > to
> > still keep the dirs and files there?
> > 
> > > of it. If the problem is that the entries have different
> > > timestamps
> > > on
> > > different builds, then the question is not limited to /dev but
> > > applies to any
> > > files created in postinstall scripts. I don't think the general
> > > approach should
> > > be to remove such files at the meta level.
> > 
> > The timestamps are already clamped by the rootfs postprocessing,
> > however this is a leak of host resources into the rootfs. Which
> > files
> > are under /dev depends on the devtmpfs implementation - which
> > depends
> > on the host kernel.
> > 
> > I propose to just clear these directories.
> 
> Thanks Felix for the details,
> 
> 1. Regarding /dev, I agree that it looks like an artifact of
> bootstrapping and
>    installation. However, I don't see that the contents are copied
> from
>    devtmpfs; could you please provide a sample command line and the
> list of
>    files to confirm the host leak?

I checked again and even if I add files / dirs to the hosts (or kas
containers) /dev, these do not show up in the bootstrapped rootfs. By
that, leaving the files should be fine from a reproducible-builds
perspective.

> 
>    After both debootstrap and d-i, I see the fixed list of console,
> fd, full,
>    null, ptmx, pts, random, shm, stderr, stdin, stdout, tty, urandom,
> zero. I
>    do think we can safely remove /dev/* for our use cases (although
>    file-hierarchy(7) says devtmpfs is only *usually* mounted), but
> I'd like to
>    document the reason for removal if we later find out that some
> software has
>    different expectations.
> 
>    We see different tools expecting at least /dev/null, /proc,
> /dev/pts and
>    /dev/shm and I also couldn't find any documentation on this. In
> the good
>    case, they fail (sometimes with cryptic messages), but often they
> crash or
>    hog the CPU (which is fun to debug from under e.g. CI + qemu).
> Some
>    indirectly related examples and discussion:
>   
> https://salsa.debian.org/installer-team/debootstrap/-/commit/a10e577675896b693c66ac77eace7ff76199da2c
>    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=571136
> 
> 2. For /run, I'd still like to see the list of the files, because I'd
> expect
>    Isar to leave it in a clean (= empty) state; maybe we have
> something that we
>    should clean up elsewhere in Isar.

For /run, the list also seems to be stable and it only contains
directories. However I don't really know where this is from. The
bootstrap is left with just /run/lock , /run/mount.

tar -tvf isar-image-base-debian-bookworm-qemuamd64.tar | grep "./run/"
lrwxrwxrwx root/root         0 2024-03-04 16:14 ./var/lock -> /run/lock
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/sendsigs.omit.d/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/log/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/user/
-rw-r--r-- root/root         0 2024-03-04 16:14 ./run/adduser
drwxrwxrwt root/root         0 2024-03-04 16:14 ./run/lock/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/lock/subsys/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/mount/
lrwxrwxrwx root/root         0 2024-03-04 16:14 ./run/shm -> /dev/shm
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/runit/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/runit/supervise/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/machines/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/ask-
password/
drwxr-xr-x 998/998           0 2024-03-04 16:14 ./run/systemd/netif/
drwxr-xr-x 998/998           0 2024-03-04 16:14
./run/systemd/netif/links/
drwxr-xr-x 998/998           0 2024-03-04 16:14
./run/systemd/netif/lldp/
drwxr-xr-x 998/998           0 2024-03-04 16:14
./run/systemd/netif/leases/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/seats/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/sessions/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/users/
drwxr-xr-x root/root         0 2024-03-04 16:14 ./run/systemd/shutdown/

Felix

> 
> 3. Regarding "vmlinux old", I assume it's about /vmlinuz.old (and I
> suggest to
>    update the commit title for better understanding -- or we can do
> that when
>    applying if it's ok for you) -- that should disappear by itself
> when we
>    multistrap the final rootfs directly from multiple apt repos. I'm
> fine
>    applying this part.
> 
> With kind regards,
> Baurzhan

Patch

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 98741da0..4af0d448 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -427,12 +427,17 @@  do_rootfs_finalize() {
             rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
 
         mountpoint -q '${ROOTFSDIR}/dev' && \
-            umount -l ${ROOTFSDIR}/dev
+            umount -l -R ${ROOTFSDIR}/dev && \
+            rm -rf ${ROOTFSDIR}/dev/*
+
         mountpoint -q '${ROOTFSDIR}/proc' && \
             umount -l ${ROOTFSDIR}/proc
         mountpoint -q '${ROOTFSDIR}/sys' && \
             umount -l ${ROOTFSDIR}/sys
 
+        rm -rf ${ROOTFSDIR}/run/*
+        rm -f ${ROOTFSDIR}/vmlinuz.old
+
         if [ -e "${ROOTFSDIR}/etc/apt/sources-list" ]; then
             mv "${ROOTFSDIR}/etc/apt/sources-list" \
                 "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"