public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH] Avoid sharing of /dev/shm from the build context
@ 2022-03-17 17:37 Jan Kiszka
  2022-03-17 18:05 ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2022-03-17 17:37 UTC (permalink / raw)
  To: isar-users; +Cc: Guillaume Pais, Florian Bezdeka

From: Jan Kiszka <jan.kiszka@siemens.com>

By bind-mounting complete /dev into the various chroots, we also share
the host instance of /dev/shm between them. If some package installation
should actually make use of that tmpfs instance, it may find content of
others there. That is at least not desirable, in few cases even
problematic (sysrepo package uses it during postinst, and this causes
troubles when multiple images are built in parallel).

This decouples all instances by mounting new instances over the
bind-mounted ones.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 meta/classes/buildchroot.bbclass                    | 3 ++-
 meta/classes/rootfs.bbclass                         | 3 ++-
 meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
index dd8f4206..3b214c6c 100644
--- a/meta/classes/buildchroot.bbclass
+++ b/meta/classes/buildchroot.bbclass
@@ -42,7 +42,8 @@ buildchroot_do_mounts() {
                 mount --bind '${CCACHE_DIR}' '${BUILDCHROOT_DIR}/ccache'
         fi
         mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
-            mount --rbind /dev '${BUILDCHROOT_DIR}/dev'
+            ( mount --rbind /dev '${BUILDCHROOT_DIR}/dev';
+              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' )
         mount --make-rslave '${BUILDCHROOT_DIR}/dev'
         mountpoint -q '${BUILDCHROOT_DIR}/proc' ||
             mount -t proc none '${BUILDCHROOT_DIR}/proc'
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 927af13f..5dd40d93 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -34,7 +34,8 @@ rootfs_do_mounts() {
     sudo -s <<'EOSUDO'
         set -e
         mountpoint -q '${ROOTFSDIR}/dev' || \
-            mount --rbind /dev '${ROOTFSDIR}/dev'
+            ( mount --rbind /dev '${ROOTFSDIR}/dev';
+              mount -t tmpfs none '${ROOTFSDIR}/dev/shm' )
         mount --make-rslave '${ROOTFSDIR}/dev'
         mountpoint -q '${ROOTFSDIR}/proc' || \
             mount -t proc none '${ROOTFSDIR}/proc'
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 1b16f874..7d94ede1 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -361,6 +361,7 @@ do_bootstrap() {
 
         # update APT
         mount --rbind /dev ${ROOTFSDIR}/dev
+        mount -t tmpfs none "${ROOTFSDIR}/dev/shm"
         mount --make-rslave ${ROOTFSDIR}/dev
         mount -t proc none ${ROOTFSDIR}/proc
         mount --rbind /sys ${ROOTFSDIR}/sys
@@ -381,6 +382,7 @@ do_bootstrap() {
         chroot "${ROOTFSDIR}" /usr/bin/apt-get dist-upgrade -y \
                                 -o Debug::pkgProblemResolver=yes
 
+        umount -l "${ROOTFSDIR}/dev/shm"
         umount -l "${ROOTFSDIR}/dev"
         umount -l "${ROOTFSDIR}/proc"
         umount -l "${ROOTFSDIR}/sys"
-- 
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Avoid sharing of /dev/shm from the build context
  2022-03-17 17:37 [PATCH] Avoid sharing of /dev/shm from the build context Jan Kiszka
@ 2022-03-17 18:05 ` Jan Kiszka
  2022-03-18  9:13   ` Baurzhan Ismagulov
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2022-03-17 18:05 UTC (permalink / raw)
  To: isar-users, Uladzimir Bely; +Cc: Guillaume Pais, Florian Bezdeka

On 17.03.22 18:37, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> By bind-mounting complete /dev into the various chroots, we also share
> the host instance of /dev/shm between them. If some package installation
> should actually make use of that tmpfs instance, it may find content of
> others there. That is at least not desirable, in few cases even
> problematic (sysrepo package uses it during postinst, and this causes
> troubles when multiple images are built in parallel).
> 
> This decouples all instances by mounting new instances over the
> bind-mounted ones.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  meta/classes/buildchroot.bbclass                    | 3 ++-
>  meta/classes/rootfs.bbclass                         | 3 ++-
>  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 2 ++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
> index dd8f4206..3b214c6c 100644
> --- a/meta/classes/buildchroot.bbclass
> +++ b/meta/classes/buildchroot.bbclass
> @@ -42,7 +42,8 @@ buildchroot_do_mounts() {
>                  mount --bind '${CCACHE_DIR}' '${BUILDCHROOT_DIR}/ccache'
>          fi
>          mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
> -            mount --rbind /dev '${BUILDCHROOT_DIR}/dev'
> +            ( mount --rbind /dev '${BUILDCHROOT_DIR}/dev';
> +              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' )
>          mount --make-rslave '${BUILDCHROOT_DIR}/dev'
>          mountpoint -q '${BUILDCHROOT_DIR}/proc' ||
>              mount -t proc none '${BUILDCHROOT_DIR}/proc'
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index 927af13f..5dd40d93 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -34,7 +34,8 @@ rootfs_do_mounts() {
>      sudo -s <<'EOSUDO'
>          set -e
>          mountpoint -q '${ROOTFSDIR}/dev' || \
> -            mount --rbind /dev '${ROOTFSDIR}/dev'
> +            ( mount --rbind /dev '${ROOTFSDIR}/dev';
> +              mount -t tmpfs none '${ROOTFSDIR}/dev/shm' )
>          mount --make-rslave '${ROOTFSDIR}/dev'
>          mountpoint -q '${ROOTFSDIR}/proc' || \
>              mount -t proc none '${ROOTFSDIR}/proc'
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> index 1b16f874..7d94ede1 100644
> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> @@ -361,6 +361,7 @@ do_bootstrap() {
>  
>          # update APT
>          mount --rbind /dev ${ROOTFSDIR}/dev
> +        mount -t tmpfs none "${ROOTFSDIR}/dev/shm"
>          mount --make-rslave ${ROOTFSDIR}/dev
>          mount -t proc none ${ROOTFSDIR}/proc
>          mount --rbind /sys ${ROOTFSDIR}/sys
> @@ -381,6 +382,7 @@ do_bootstrap() {
>          chroot "${ROOTFSDIR}" /usr/bin/apt-get dist-upgrade -y \
>                                  -o Debug::pkgProblemResolver=yes
>  
> +        umount -l "${ROOTFSDIR}/dev/shm"
>          umount -l "${ROOTFSDIR}/dev"
>          umount -l "${ROOTFSDIR}/proc"
>          umount -l "${ROOTFSDIR}/sys"

Uladzimir, didn't check if this is going to be completely obsoleted by
sbuild. If so, this can be ignored, and we will carry it locally until
sbuild is merged.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Avoid sharing of /dev/shm from the build context
  2022-03-17 18:05 ` Jan Kiszka
@ 2022-03-18  9:13   ` Baurzhan Ismagulov
  2022-03-18 13:08     ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Baurzhan Ismagulov @ 2022-03-18  9:13 UTC (permalink / raw)
  To: isar-users

On Thu, Mar 17, 2022 at 07:05:16PM +0100, Jan Kiszka wrote:
> > diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
> > index dd8f4206..3b214c6c 100644
> > --- a/meta/classes/buildchroot.bbclass
> > +++ b/meta/classes/buildchroot.bbclass
> > @@ -42,7 +42,8 @@ buildchroot_do_mounts() {
> >                  mount --bind '${CCACHE_DIR}' '${BUILDCHROOT_DIR}/ccache'
> >          fi
> >          mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
> > -            mount --rbind /dev '${BUILDCHROOT_DIR}/dev'
> > +            ( mount --rbind /dev '${BUILDCHROOT_DIR}/dev';
> > +              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' )
> >          mount --make-rslave '${BUILDCHROOT_DIR}/dev'

I'd personally convert that to if because I never remember the failure behavior
in subshells. While at it, I'd also combine to a single mount -o rbind,rslave
to avoid potential shared mount windows.

I think that mounting every directory explicitly (bind instead of rbind)
provides more control and avoids a bunch of issues with recursive mounts.


> Uladzimir, didn't check if this is going to be completely obsoleted by
> sbuild. If so, this can be ignored, and we will carry it locally until
> sbuild is merged.

According to Uladzimir, ATM this is orthogonal and should be applied, we keep
buildchroot at least for some time, as e.g. wic is using it.


With kind regards,
Baurzhan.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Avoid sharing of /dev/shm from the build context
  2022-03-18  9:13   ` Baurzhan Ismagulov
@ 2022-03-18 13:08     ` Jan Kiszka
  2022-03-18 14:53       ` Baurzhan Ismagulov
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2022-03-18 13:08 UTC (permalink / raw)
  To: isar-users

On 18.03.22 10:13, Baurzhan Ismagulov wrote:
> On Thu, Mar 17, 2022 at 07:05:16PM +0100, Jan Kiszka wrote:
>>> diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
>>> index dd8f4206..3b214c6c 100644
>>> --- a/meta/classes/buildchroot.bbclass
>>> +++ b/meta/classes/buildchroot.bbclass
>>> @@ -42,7 +42,8 @@ buildchroot_do_mounts() {
>>>                  mount --bind '${CCACHE_DIR}' '${BUILDCHROOT_DIR}/ccache'
>>>          fi
>>>          mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
>>> -            mount --rbind /dev '${BUILDCHROOT_DIR}/dev'
>>> +            ( mount --rbind /dev '${BUILDCHROOT_DIR}/dev';
>>> +              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' )
>>>          mount --make-rslave '${BUILDCHROOT_DIR}/dev'
> 
> I'd personally convert that to if because I never remember the failure behavior

You mean && - sure.

> in subshells. While at it, I'd also combine to a single mount -o rbind,rslave
> to avoid potential shared mount windows.
> 
> I think that mounting every directory explicitly (bind instead of rbind)
> provides more control and avoids a bunch of issues with recursive mounts.
> 

Do we know what all needs to be mounted then?

> 
>> Uladzimir, didn't check if this is going to be completely obsoleted by
>> sbuild. If so, this can be ignored, and we will carry it locally until
>> sbuild is merged.
> 
> According to Uladzimir, ATM this is orthogonal and should be applied, we keep
> buildchroot at least for some time, as e.g. wic is using it.
> 

OK.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Avoid sharing of /dev/shm from the build context
  2022-03-18 13:08     ` Jan Kiszka
@ 2022-03-18 14:53       ` Baurzhan Ismagulov
  2022-03-18 15:46         ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Baurzhan Ismagulov @ 2022-03-18 14:53 UTC (permalink / raw)
  To: isar-users

On Fri, Mar 18, 2022 at 02:08:50PM +0100, Jan Kiszka wrote:
> > I'd personally convert that to if because I never remember the failure behavior
> 
> You mean && - sure.

Or like this.


> > in subshells. While at it, I'd also combine to a single mount -o rbind,rslave
> > to avoid potential shared mount windows.
> > 
> > I think that mounting every directory explicitly (bind instead of rbind)
> > provides more control and avoids a bunch of issues with recursive mounts.
> 
> Do we know what all needs to be mounted then?

As far as I remember, /dev and /dev/shm were necessary for building; /dev/pts
was only necessary for interactive work (e.g. sudo).


With kind regards,
Baurzhan.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Avoid sharing of /dev/shm from the build context
  2022-03-18 14:53       ` Baurzhan Ismagulov
@ 2022-03-18 15:46         ` Jan Kiszka
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2022-03-18 15:46 UTC (permalink / raw)
  To: isar-users

On 18.03.22 15:53, Baurzhan Ismagulov wrote:
> On Fri, Mar 18, 2022 at 02:08:50PM +0100, Jan Kiszka wrote:
>>> I'd personally convert that to if because I never remember the failure behavior
>>
>> You mean && - sure.
> 
> Or like this.
> 
> 
>>> in subshells. While at it, I'd also combine to a single mount -o rbind,rslave
>>> to avoid potential shared mount windows.
>>>
>>> I think that mounting every directory explicitly (bind instead of rbind)
>>> provides more control and avoids a bunch of issues with recursive mounts.
>>
>> Do we know what all needs to be mounted then?
> 
> As far as I remember, /dev and /dev/shm were necessary for building; /dev/pts
> was only necessary for interactive work (e.g. sudo).
> 

OK, then let's drop the rest and see what explodes. ;)

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-03-18 15:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-17 17:37 [PATCH] Avoid sharing of /dev/shm from the build context Jan Kiszka
2022-03-17 18:05 ` Jan Kiszka
2022-03-18  9:13   ` Baurzhan Ismagulov
2022-03-18 13:08     ` Jan Kiszka
2022-03-18 14:53       ` Baurzhan Ismagulov
2022-03-18 15:46         ` Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox