Re: [OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path

["Paul Barker" <pbarker@konsulko.com>]

On Fri, 3 Apr 2020 21:53:39 +0200
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:

> ping?

I think that '../pseudo' should not be used here. I'll explain inline...

> >
> > This results in a rootfs owned by the user that is running the wic
> > command (usually UID 1000), which makes some rootfs unbootable.
> >
> > To fix this we copy the content of the pseudo folders to the new folder
> > and modify the pseudo database using the "pseudo -B" command.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > ---
> >  scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > index 705aeb5563..40419a64b3 100644
> > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > @@ -16,11 +16,11 @@ import os
> >  import shutil
> >  import sys
> >
> > -from oe.path import copyhardlinktree
> > +from oe.path import copyhardlinktree, copytree
> >
> >  from wic import WicError
> >  from wic.pluginbase import SourcePlugin
> > -from wic.misc import get_bitbake_var
> > +from wic.misc import get_bitbake_var, exec_native_cmd
> >
> >  logger = logging.getLogger('wic')
> >
> > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> >
> >          return os.path.realpath(image_rootfs_dir)
> >
> > +    @staticmethod
> > +    def __get_pseudo(native_sysroot, rootfs):
> > +        pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > +        pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > +        pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > +        pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > +        pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > +        return pseudo
> > +
> >      @classmethod
> >      def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> >                               oe_builddir, bootimg_dir, kernel_dir,
> > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> >
> >              if os.path.lexists(new_rootfs):
> >                  shutil.rmtree(os.path.join(new_rootfs))
> > -
> >              copyhardlinktree(part.rootfs_dir, new_rootfs)
> >
> > +            if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):

new_rootfs is set by the following statement a few lines above:

    new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))

Consider that `cr_workdir` may contain multiple rootfs staging directories
corresponding to multiple lines in the wks file, for example if a rootfs
image is duplicated into multiple partitions for redundancy. In that case
`os.path.join(new_rootfs, "../pseudo")` will clash between these different
rootfs copies.

Let's use an explicit path instead, such as:

    new_pseudo = os.path.realpath(os.path.join(cr_workdir, "pseudo%d" % part.lineno))

> > +                shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > +            copytree(os.path.join(part.rootfs_dir, "../pseudo"),

part.rootfs_dir is whatever is given as the option to `--rootfs-dir`. There
is no guarantee that `../psuedo` is valid or if it corresponds to the rootfs
directory given. It's unsafe to step up the directory tree and make
assumptions like this.

> > +                     os.path.join(new_rootfs, "../pseudo"))  
> > +            pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > +                                                part.rootfs_dir, new_rootfs)
> > +            exec_native_cmd(pseudo_cmd, native_sysroot)
> > +
> >              for path in part.include_path or []:
> >                  copyhardlinktree(path, new_rootfs)  

-- 
Paul Barker
Konsulko Group