* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it [not found] <20150901104522.9FC59505A9@opal.openembedded.org> @ 2015-09-01 19:09 ` Martin Jansa 2015-09-01 23:11 ` Alex Franco 2015-09-03 11:59 ` Richard Purdie 0 siblings, 2 replies; 6+ messages in thread From: Martin Jansa @ 2015-09-01 19:09 UTC (permalink / raw) To: openembedded-core, Alex Franco; +Cc: openembedded-commits [-- Attachment #1: Type: text/plain, Size: 3958 bytes --] On Tue, Sep 01, 2015 at 10:45:22AM +0000, git@git.openembedded.org wrote: > Module: openembedded-core.git > Branch: master > Commit: 8236d57439640a185c0226312cd4903a3ce2f53b > URL: http://git.openembedded.org/?p=openembedded-core.git&a=commit;h=8236d57439640a185c0226312cd4903a3ce2f53b > > Author: Alex Franco <alejandro.franco@linux.intel.com> > Date: Fri Aug 28 17:34:04 2015 -0500 > > Fix mode +st on TMPDIR when creating it > > A sanity check fails when TMPDIR has setuid, setgid set. It was > proposed to fix this on TMPDIR creation instead of failing with > a sanity test only. This patch handles removal of those special > bits (and additonally, the sticky bit) from BUILDDIR and TMPDIR > when these directories are created. Can you explain why it's needed to remove them? I have tmp-glibc mounted as tmpfs: drwxrwxrwt 4 root jenkins 80 Sep 1 19:02 tmp-glibc and since this change all bitbake executions are failing with: NOTE: Started PRServer with DBfile: /home/jenkins/oe/world/shr-core/cache/prserv.sqlite3, IP: 127.0.0.1, PORT: 43102, PID: 9220 ERROR: Execution of event handler 'check_sanity_eventhandler' failed Traceback (most recent call last): File "check_sanity_eventhandler(e)", line 6, in check_sanity_eventhandler(e=<bb.event.SanityCheck object at 0x3c76ad0>) File "sanity.bbclass", line 34, in check_sanity(sanity_data=<bb.data_smart.DataSmart object at 0x3c83890>) File "sanity.bbclass", line 157, in check_sanity_everybuild(status=<SanityStatus object at 0x3c83850>, d=<bb.data_smart.DataSmart object at 0x3c83890>) OSError: [Errno 1] Operation not permitted: '/home/jenkins/oe/world/shr-core/tmp-glibc' I can change the permissions manually, but I would like to know why is this needed and checked every time the bitbake is executed. Regards, > > [YOCTO #7669] > > Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > > --- > > meta/classes/sanity.bbclass | 5 +++++ > scripts/oe-setup-builddir | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass > index ef90fc8..2864318 100644 > --- a/meta/classes/sanity.bbclass > +++ b/meta/classes/sanity.bbclass > @@ -686,6 +686,7 @@ def check_sanity_version_change(status, d): > status.addresult(check_not_nfs(tmpdir, "TMPDIR")) > > def check_sanity_everybuild(status, d): > + import os, stat > # Sanity tests which test the users environment so need to run at each build (or are so cheap > # it makes sense to always run them. > > @@ -839,6 +840,10 @@ def check_sanity_everybuild(status, d): > status.addresult("Error, TMPDIR has changed location. You need to either move it back to %s or rebuild\n" % saved_tmpdir) > else: > bb.utils.mkdirhier(tmpdir) > + # Remove setuid, setgid and sticky bits from TMPDIR > + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID) > + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID) > + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX) > with open(checkfile, "w") as f: > f.write(tmpdir) > > diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir > index add0b50..f5b7e4e 100755 > --- a/scripts/oe-setup-builddir > +++ b/scripts/oe-setup-builddir > @@ -24,6 +24,7 @@ if [ -z "$BUILDDIR" ]; then > fi > > mkdir -p "$BUILDDIR/conf" > +chmod -R -st "$BUILDDIR" > > if [ ! -d "$BUILDDIR" ]; then > echo >&2 "Error: The builddir ($BUILDDIR) does not exist!" > > -- > _______________________________________________ > Openembedded-commits mailing list > Openembedded-commits@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-commits -- Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 188 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it 2015-09-01 19:09 ` [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it Martin Jansa @ 2015-09-01 23:11 ` Alex Franco 2015-09-01 23:47 ` Christopher Larson 2015-09-03 11:59 ` Richard Purdie 1 sibling, 1 reply; 6+ messages in thread From: Alex Franco @ 2015-09-01 23:11 UTC (permalink / raw) To: Martin Jansa, openembedded-core; +Cc: openembedded-commits Hi Martin, I gather the original reason (for the sanity test, as well as these changes) is that it is better for everything under BUILDDIR - and certainly TMPDIR to be owned by the user running bitbake and have standard permissions, the sticky bit itself would prevent a user running bitbake, even with +rwx permissions, from making changes to items it might need to manipulate, if those are not owned by this user. Similarly setuid and setgid might propagate user/group ownerships different from the user bitbake is running as, into BUILDDIR and TMPDIR (and items therein). For more info take a look at: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7669 Perhaps there are more pressing reasons, perhaps Christopher Larson can comment a bit on this. Alex Franco On 09/01/2015 02:09 PM, Martin Jansa wrote: > On Tue, Sep 01, 2015 at 10:45:22AM +0000, git@git.openembedded.org wrote: >> Module: openembedded-core.git >> Branch: master >> Commit: 8236d57439640a185c0226312cd4903a3ce2f53b >> URL: http://git.openembedded.org/?p=openembedded-core.git&a=commit;h=8236d57439640a185c0226312cd4903a3ce2f53b >> >> Author: Alex Franco <alejandro.franco@linux.intel.com> >> Date: Fri Aug 28 17:34:04 2015 -0500 >> >> Fix mode +st on TMPDIR when creating it >> >> A sanity check fails when TMPDIR has setuid, setgid set. It was >> proposed to fix this on TMPDIR creation instead of failing with >> a sanity test only. This patch handles removal of those special >> bits (and additonally, the sticky bit) from BUILDDIR and TMPDIR >> when these directories are created. > Can you explain why it's needed to remove them? > > I have tmp-glibc mounted as tmpfs: > drwxrwxrwt 4 root jenkins 80 Sep 1 19:02 tmp-glibc > > and since this change all bitbake executions are failing with: > > NOTE: Started PRServer with DBfile: /home/jenkins/oe/world/shr-core/cache/prserv.sqlite3, IP: 127.0.0.1, PORT: 43102, PID: 9220 > ERROR: Execution of event handler 'check_sanity_eventhandler' failed > Traceback (most recent call last): > File "check_sanity_eventhandler(e)", line 6, in check_sanity_eventhandler(e=<bb.event.SanityCheck object at 0x3c76ad0>) > File "sanity.bbclass", line 34, in check_sanity(sanity_data=<bb.data_smart.DataSmart object at 0x3c83890>) > File "sanity.bbclass", line 157, in check_sanity_everybuild(status=<SanityStatus object at 0x3c83850>, d=<bb.data_smart.DataSmart object at 0x3c83890>) > OSError: [Errno 1] Operation not permitted: '/home/jenkins/oe/world/shr-core/tmp-glibc' > > I can change the permissions manually, but I would like to know > why is this needed and checked every time the bitbake is executed. > > Regards, > >> [YOCTO #7669] >> >> Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com> >> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> >> >> --- >> >> meta/classes/sanity.bbclass | 5 +++++ >> scripts/oe-setup-builddir | 1 + >> 2 files changed, 6 insertions(+) >> >> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass >> index ef90fc8..2864318 100644 >> --- a/meta/classes/sanity.bbclass >> +++ b/meta/classes/sanity.bbclass >> @@ -686,6 +686,7 @@ def check_sanity_version_change(status, d): >> status.addresult(check_not_nfs(tmpdir, "TMPDIR")) >> >> def check_sanity_everybuild(status, d): >> + import os, stat >> # Sanity tests which test the users environment so need to run at each build (or are so cheap >> # it makes sense to always run them. >> >> @@ -839,6 +840,10 @@ def check_sanity_everybuild(status, d): >> status.addresult("Error, TMPDIR has changed location. You need to either move it back to %s or rebuild\n" % saved_tmpdir) >> else: >> bb.utils.mkdirhier(tmpdir) >> + # Remove setuid, setgid and sticky bits from TMPDIR >> + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID) >> + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID) >> + os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX) >> with open(checkfile, "w") as f: >> f.write(tmpdir) >> >> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir >> index add0b50..f5b7e4e 100755 >> --- a/scripts/oe-setup-builddir >> +++ b/scripts/oe-setup-builddir >> @@ -24,6 +24,7 @@ if [ -z "$BUILDDIR" ]; then >> fi >> >> mkdir -p "$BUILDDIR/conf" >> +chmod -R -st "$BUILDDIR" >> >> if [ ! -d "$BUILDDIR" ]; then >> echo >&2 "Error: The builddir ($BUILDDIR) does not exist!" >> >> -- >> _______________________________________________ >> Openembedded-commits mailing list >> Openembedded-commits@lists.openembedded.org >> http://lists.openembedded.org/mailman/listinfo/openembedded-commits ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it 2015-09-01 23:11 ` Alex Franco @ 2015-09-01 23:47 ` Christopher Larson 2015-09-02 11:14 ` Richard Purdie 0 siblings, 1 reply; 6+ messages in thread From: Christopher Larson @ 2015-09-01 23:47 UTC (permalink / raw) To: Alex Franco Cc: openembedded-commits, Patches and discussions about the oe-core layer [-- Attachment #1: Type: text/plain, Size: 1263 bytes --] On Tue, Sep 1, 2015 at 4:11 PM, Alex Franco < alejandro.franco@linux.intel.com> wrote: > I gather the original reason (for the sanity test, as well as these > changes) is that it is better for everything under BUILDDIR - and certainly > TMPDIR to be owned by the user running bitbake and have standard > permissions, the sticky bit itself would prevent a user running bitbake, > even with +rwx permissions, from making changes to items it might need to > manipulate, if those are not owned by this user. Similarly setuid and > setgid might propagate user/group ownerships different from the user > bitbake is running as, into BUILDDIR and TMPDIR (and items therein). > > For more info take a look at: > https://bugzilla.yoctoproject.org/show_bug.cgi?id=7669 > > Perhaps there are more pressing reasons, perhaps Christopher Larson can > comment a bit on this. > Indeed, the g+s can flow down into the files put into the packages, and then into the rootfs. I ran into a this problem quite a bit, since my project areas are in a g+s area to keep the files accessible to the group. -- Christopher Larson clarson at kergoth dot com Founder - BitBake, OpenEmbedded, OpenZaurus Maintainer - Tslib Senior Software Engineer, Mentor Graphics [-- Attachment #2: Type: text/html, Size: 1866 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it 2015-09-01 23:47 ` Christopher Larson @ 2015-09-02 11:14 ` Richard Purdie 2015-09-02 16:37 ` Alex Franco 0 siblings, 1 reply; 6+ messages in thread From: Richard Purdie @ 2015-09-02 11:14 UTC (permalink / raw) To: Christopher Larson Cc: Patches and discussions about the oe-core layer, openembedded-commits On Tue, 2015-09-01 at 16:47 -0700, Christopher Larson wrote: > > On Tue, Sep 1, 2015 at 4:11 PM, Alex Franco > <alejandro.franco@linux.intel.com> wrote: > I gather the original reason (for the sanity test, as well as > these changes) is that it is better for everything under > BUILDDIR - and certainly TMPDIR to be owned by the user > running bitbake and have standard permissions, the sticky bit > itself would prevent a user running bitbake, even with +rwx > permissions, from making changes to items it might need to > manipulate, if those are not owned by this user. Similarly > setuid and setgid might propagate user/group ownerships > different from the user bitbake is running as, into BUILDDIR > and TMPDIR (and items therein). > > For more info take a look at: > https://bugzilla.yoctoproject.org/show_bug.cgi?id=7669 > > Perhaps there are more pressing reasons, perhaps Christopher > Larson can comment a bit on this. > > Indeed, the g+s can flow down into the files put into the packages, > and then into the rootfs. I ran into a this problem quite a bit, since > my project areas are in a g+s area to keep the files accessible to the > group. There is actually another problem here. The -R means if you do this to an existing build directory, it takes around 10 mins on the machine I just rebooted with a cold cache. We need to lose the -R there. Cheers, Richard ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it 2015-09-02 11:14 ` Richard Purdie @ 2015-09-02 16:37 ` Alex Franco 0 siblings, 0 replies; 6+ messages in thread From: Alex Franco @ 2015-09-02 16:37 UTC (permalink / raw) To: Richard Purdie, Christopher Larson Cc: Patches and discussions about the oe-core layer, openembedded-commits Thanks, I have sent a patch after removing the -R and testing a bit Alex Franco On 09/02/2015 06:14 AM, Richard Purdie wrote: > On Tue, 2015-09-01 at 16:47 -0700, Christopher Larson wrote: >> On Tue, Sep 1, 2015 at 4:11 PM, Alex Franco >> <alejandro.franco@linux.intel.com> wrote: >> I gather the original reason (for the sanity test, as well as >> these changes) is that it is better for everything under >> BUILDDIR - and certainly TMPDIR to be owned by the user >> running bitbake and have standard permissions, the sticky bit >> itself would prevent a user running bitbake, even with +rwx >> permissions, from making changes to items it might need to >> manipulate, if those are not owned by this user. Similarly >> setuid and setgid might propagate user/group ownerships >> different from the user bitbake is running as, into BUILDDIR >> and TMPDIR (and items therein). >> >> For more info take a look at: >> https://bugzilla.yoctoproject.org/show_bug.cgi?id=7669 >> >> Perhaps there are more pressing reasons, perhaps Christopher >> Larson can comment a bit on this. >> >> Indeed, the g+s can flow down into the files put into the packages, >> and then into the rootfs. I ran into a this problem quite a bit, since >> my project areas are in a g+s area to keep the files accessible to the >> group. > > There is actually another problem here. The -R means if you do this to > an existing build directory, it takes around 10 mins on the machine I > just rebooted with a cold cache. We need to lose the -R there. > > Cheers, > > Richard > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it 2015-09-01 19:09 ` [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it Martin Jansa 2015-09-01 23:11 ` Alex Franco @ 2015-09-03 11:59 ` Richard Purdie 1 sibling, 0 replies; 6+ messages in thread From: Richard Purdie @ 2015-09-03 11:59 UTC (permalink / raw) To: Martin Jansa; +Cc: openembedded-commits, openembedded-core On Tue, 2015-09-01 at 21:09 +0200, Martin Jansa wrote: > On Tue, Sep 01, 2015 at 10:45:22AM +0000, git@git.openembedded.org wrote: > > Module: openembedded-core.git > > Branch: master > > Commit: 8236d57439640a185c0226312cd4903a3ce2f53b > > URL: http://git.openembedded.org/?p=openembedded-core.git&a=commit;h=8236d57439640a185c0226312cd4903a3ce2f53b > > > > Author: Alex Franco <alejandro.franco@linux.intel.com> > > Date: Fri Aug 28 17:34:04 2015 -0500 > > > > Fix mode +st on TMPDIR when creating it > > > > A sanity check fails when TMPDIR has setuid, setgid set. It was > > proposed to fix this on TMPDIR creation instead of failing with > > a sanity test only. This patch handles removal of those special > > bits (and additonally, the sticky bit) from BUILDDIR and TMPDIR > > when these directories are created. > > Can you explain why it's needed to remove them? At least in theory the sticky bit (t) can end up influencing the target package file ownership. In the interests of build determinism we really need TMPDIR to start in a consistent way with a consistent set of permissions. Put another way, I don't really want to have to add new sanity tests that test if builds work the same if TMPDIR is +t compared to a dir that isn't set that way. So if you could tweak your autobuilder setup to lose the sticky bit on that directory, I think that would be preferable to adding more corner cases to the build environment. If everyone had setups like this, that would change my view but it doesn't seem to eb the ase. Cheers, Richard ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-03 11:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20150901104522.9FC59505A9@opal.openembedded.org>
2015-09-01 19:09 ` [oe-commits] Alex Franco : Fix mode +st on TMPDIR when creating it Martin Jansa
2015-09-01 23:11 ` Alex Franco
2015-09-01 23:47 ` Christopher Larson
2015-09-02 11:14 ` Richard Purdie
2015-09-02 16:37 ` Alex Franco
2015-09-03 11:59 ` Richard Purdie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox