[PATCH] sstate: another fix for touching files inside pseudo

[PATCH] sstate: another fix for touching files inside pseudo

[Jose Quaresma]

This patch is a fixup for 676757f "sstate: fix touching files inside pseudo"

running the 'id' command inside the sstate_unpack_package
function shows that this funcion run inside the pseudo:

 uid=0(root) gid=0(root) groups=0(root)

The check for [ -w ${SSTATE_PKG} ] and [ -O ${SSTATE_PKG}.siginfo ]
will always return true and the touch can fail when the real user
don't have permission or in readonly filesystem.

As the documentation refers:
- the file test operator "-w" check if the file has write permission
(for the user running the test).
- the file test operator "-O" check if you are owner of file

We can avoid this test running the touch and mask any return errors
that we have.

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 8182010047..e4d58639f3 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -900,12 +900,12 @@ sstate_unpack_package () {
 	fi
 
 	tar -I "$ZSTD" -xvf ${SSTATE_PKG}
-	# update .siginfo atime on local/NFS mirror
-	[ -O ${SSTATE_PKG}.siginfo ] && [ -w ${SSTATE_PKG}.siginfo ] && [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
-	# Use "! -w ||" to return true for read only files
-	[ ! -w ${SSTATE_PKG} ] || touch --no-dereference ${SSTATE_PKG}
-	[ ! -w ${SSTATE_PKG}.sig ] || [ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig
-	[ ! -w ${SSTATE_PKG}.siginfo ] || [ ! -e ${SSTATE_PKG}.siginfo ] || touch --no-dereference ${SSTATE_PKG}.siginfo
+	# update .siginfo atime on local/NFS mirror if it is a symbolic link
+	[ ! -h ${SSTATE_PKG}.siginfo ] || touch -a ${SSTATE_PKG}.siginfo 2>/dev/null || true
+	# update each symbolic link instead of any referenced file
+	touch --no-dereference ${SSTATE_PKG} 2>/dev/null || true
+	touch --no-dereference ${SSTATE_PKG}.siginfo 2>/dev/null || true
+	[ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig 2>/dev/null || true
 }
 
 BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
-- 
2.33.1

RE: [OE-core] [PATCH] sstate: another fix for touching files inside pseudo

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Jose Quaresma
> Sent: den 4 november 2021 16:42
> To: openembedded-core@lists.openembedded.org
> Cc: Jose Quaresma <quaresma.jose@gmail.com>
> Subject: [OE-core] [PATCH] sstate: another fix for touching files inside
> pseudo
> 
> This patch is a fixup for 676757f "sstate: fix touching files inside
> pseudo"
> 
> running the 'id' command inside the sstate_unpack_package
> function shows that this funcion run inside the pseudo:
> 
>  uid=0(root) gid=0(root) groups=0(root)
> 
> The check for [ -w ${SSTATE_PKG} ] and [ -O ${SSTATE_PKG}.siginfo ]
> will always return true and the touch can fail when the real user
> don't have permission or in readonly filesystem.
> 
> As the documentation refers:
> - the file test operator "-w" check if the file has write permission
> (for the user running the test).
> - the file test operator "-O" check if you are owner of file
> 
> We can avoid this test running the touch and mask any return errors
> that we have.
> 
> Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> ---
>  meta/classes/sstate.bbclass | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 8182010047..e4d58639f3 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -900,12 +900,12 @@ sstate_unpack_package () {
>  	fi
> 
>  	tar -I "$ZSTD" -xvf ${SSTATE_PKG}
> -	# update .siginfo atime on local/NFS mirror
> -	[ -O ${SSTATE_PKG}.siginfo ] && [ -w ${SSTATE_PKG}.siginfo ] && [ -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> -	# Use "! -w ||" to return true for read only files
> -	[ ! -w ${SSTATE_PKG} ] || touch --no-dereference ${SSTATE_PKG}
> -	[ ! -w ${SSTATE_PKG}.sig ] || [ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig
> -	[ ! -w ${SSTATE_PKG}.siginfo ] || [ ! -e ${SSTATE_PKG}.siginfo ] || touch --no-dereference ${SSTATE_PKG}.siginfo
> +	# update .siginfo atime on local/NFS mirror if it is a symbolic link
> +	[ ! -h ${SSTATE_PKG}.siginfo ] || touch -a ${SSTATE_PKG}.siginfo 2>/dev/null || true
> +	# update each symbolic link instead of any referenced file
> +	touch --no-dereference ${SSTATE_PKG} 2>/dev/null || true
> +	touch --no-dereference ${SSTATE_PKG}.siginfo 2>/dev/null || true

To match the original code this should be:

	[ ! -e ${SSTATE_PKG}.siginfo ] || touch --no-dereference ${SSTATE_PKG}.siginfo 2>/dev/null || true

Or was there some reason you removed the test for if the siginfo 
file exists before touching it? If so, then the commit message 
should state this.

> +	[ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference ${SSTATE_PKG}.sig 2>/dev/null || true
>  }
> 
>  BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
> --
> 2.33.1

//Peter

Re: [OE-core] [PATCH] sstate: another fix for touching files inside pseudo

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 3505 bytes --]

Peter Kjellerstedt <peter.kjellerstedt@axis.com> escreveu no dia quinta,
4/11/2021 à(s) 17:01:

> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-
> > core@lists.openembedded.org> On Behalf Of Jose Quaresma
> > Sent: den 4 november 2021 16:42
> > To: openembedded-core@lists.openembedded.org
> > Cc: Jose Quaresma <quaresma.jose@gmail.com>
> > Subject: [OE-core] [PATCH] sstate: another fix for touching files inside
> > pseudo
> >
> > This patch is a fixup for 676757f "sstate: fix touching files inside
> > pseudo"
> >
> > running the 'id' command inside the sstate_unpack_package
> > function shows that this funcion run inside the pseudo:
> >
> >  uid=0(root) gid=0(root) groups=0(root)
> >
> > The check for [ -w ${SSTATE_PKG} ] and [ -O ${SSTATE_PKG}.siginfo ]
> > will always return true and the touch can fail when the real user
> > don't have permission or in readonly filesystem.
> >
> > As the documentation refers:
> > - the file test operator "-w" check if the file has write permission
> > (for the user running the test).
> > - the file test operator "-O" check if you are owner of file
> >
> > We can avoid this test running the touch and mask any return errors
> > that we have.
> >
> > Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> > ---
> >  meta/classes/sstate.bbclass | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index 8182010047..e4d58639f3 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> > @@ -900,12 +900,12 @@ sstate_unpack_package () {
> >       fi
> >
> >       tar -I "$ZSTD" -xvf ${SSTATE_PKG}
> > -     # update .siginfo atime on local/NFS mirror
> > -     [ -O ${SSTATE_PKG}.siginfo ] && [ -w ${SSTATE_PKG}.siginfo ] && [
> -h ${SSTATE_PKG}.siginfo ] && touch -a ${SSTATE_PKG}.siginfo
> > -     # Use "! -w ||" to return true for read only files
> > -     [ ! -w ${SSTATE_PKG} ] || touch --no-dereference ${SSTATE_PKG}
> > -     [ ! -w ${SSTATE_PKG}.sig ] || [ ! -e ${SSTATE_PKG}.sig ] || touch
> --no-dereference ${SSTATE_PKG}.sig
> > -     [ ! -w ${SSTATE_PKG}.siginfo ] || [ ! -e ${SSTATE_PKG}.siginfo ]
> || touch --no-dereference ${SSTATE_PKG}.siginfo
> > +     # update .siginfo atime on local/NFS mirror if it is a symbolic
> link
> > +     [ ! -h ${SSTATE_PKG}.siginfo ] || touch -a ${SSTATE_PKG}.siginfo
> 2>/dev/null || true
> > +     # update each symbolic link instead of any referenced file
> > +     touch --no-dereference ${SSTATE_PKG} 2>/dev/null || true
> > +     touch --no-dereference ${SSTATE_PKG}.siginfo 2>/dev/null || true
>
> To match the original code this should be:
>
>         [ ! -e ${SSTATE_PKG}.siginfo ] || touch --no-dereference
> ${SSTATE_PKG}.siginfo 2>/dev/null || true
>
> Or was there some reason you removed the test for if the siginfo
> file exists before touching it? If so, then the commit message
> should state this.
>

I assumed that when ${SSTATE_PKG} exists the ${SSTATE_PKG}.siginfo also
exists.
You are true in your observation that this doesn't match the original code.
I will send a v2.


>
> > +     [ ! -e ${SSTATE_PKG}.sig ] || touch --no-dereference
> ${SSTATE_PKG}.sig 2>/dev/null || true
> >  }
> >
> >  BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
> > --
> > 2.33.1
>
> //Peter
>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 4948 bytes --]