All of lore.kernel.org
 help / color / mirror / Atom feed
* Directory permissions and ownership -- RFC
@ 2011-06-21 16:43 Mark Hatle
  2011-06-21 18:57 ` Phil Blundell
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Hatle @ 2011-06-21 16:43 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

I've had the task to resolve the directory permissions and ownership issues.
From the original RFC I sent out, a lot has changed.

The way OE, opkg and deb packages are defined, there is no way to define what
package "owns" a directory.  There is an expectation that directory permissions,
owner and group are consistent between packages (and thus recipes).

I've worked though a number of the issues.  While I'm not posting the patches
yet for inclusion, I'd like some feed back on the two specific patches below.

Patch 1:

Adjust the umask to 022.  This resolves the problem of dynamically generated
directories (mkdir -p) and specific files (touch foo) having odd permissions.

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621

Patch 2:

The item adds a new step to the package.bbclass, fixup_perms.  The function that
is responsible for fixing directory and file permissions, owners and groups
during the packaging process. This will fix various issues where two packages
may create the same directory and end up with different permissions, owner
and/or group.

The issue being resolved is that if two packages conflict in their ownership of
a directory, the first installed into the rootfs sets the permissions. This
leads to a least potentially non-deterministic filesystems, at worst security
defects.

The function has a set of default values. We sanitize all of the system
directories, as defined in bitbake.conf as 0755, root, root. We also have
determined as series of documentation directories. These are sanitized as 0755,
root, root with the files they contain as 0644, root, root. The user can add
their own settings, or override the defaults by providing a
meta/files/fs-perms.txt file. The format of this file is described in the
default file.

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=eb76974fb73f2793e9d6191fb12d502fefc74c80


Any comments/feedback is appreciated.

--Mark



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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 16:43 Directory permissions and ownership -- RFC Mark Hatle
@ 2011-06-21 18:57 ` Phil Blundell
  2011-06-21 19:12   ` Mark Hatle
  0 siblings, 1 reply; 16+ messages in thread
From: Phil Blundell @ 2011-06-21 18:57 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
> Adjust the umask to 022.  This resolves the problem of dynamically generated
> directories (mkdir -p) and specific files (touch foo) having odd permissions.
> 
> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621

Are you confident that this isn't going to break anything like
group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
to 022 for everything that bitbake does, although I can see that making
it be so for particular tasks like do_install() might have some merit.
Even in the latter case, though, I wonder whether we should just be
paying more attention to recipe hygiene and using "install -m ..." with
the permissions that we actually want.

I'm also not quite sure I understand why this is necessary if
fixup_perms is going to fix up the permissions anyway. 

> The user can add their own settings, or override the defaults by providing a
> meta/files/fs-perms.txt file. 

Using a magically-named file rather than metadata vars is a bit
unconventional and seems like it will make life unnecessarily harder for
distros/other overlays.  Is there any reason it needs to be done that
way?

p.





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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 18:57 ` Phil Blundell
@ 2011-06-21 19:12   ` Mark Hatle
  2011-06-21 21:09     ` Phil Blundell
                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Mark Hatle @ 2011-06-21 19:12 UTC (permalink / raw)
  To: openembedded-core

On 6/21/11 1:57 PM, Phil Blundell wrote:
> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>
>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
> 
> Are you confident that this isn't going to break anything like
> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
> to 022 for everything that bitbake does, although I can see that making
> it be so for particular tasks like do_install() might have some merit.
> Even in the latter case, though, I wonder whether we should just be
> paying more attention to recipe hygiene and using "install -m ..." with
> the permissions that we actually want.

This is why I bring this up.. I'm a bit concerned that doing it generally will
have unintended consequences.  So far I am not aware of any.  Moving it to a
different place in the process may be better.  The only issue I've found so far
is that just coding int into "do_install" really isn't an option.  Between the
custom do_install components, various classes, etc.. it's difficult in the
current infrastructure to find a centralized location to set the value.

(I'd love to be corrected if someone things of another way of doing it.)  The
setting of the umask is a very low cost operation, so doing it for certain steps
shouldn't cause a performance penalty... but until we figure that out this is
the best and easiest solution I've come up with.

> I'm also not quite sure I understand why this is necessary if
> fixup_perms is going to fix up the permissions anyway. 

fixup_perms only knows of certain global directories, it doesn't know of ones
that may or may not be unique to given packages, or sets of packages.  (yes they
could be added over time to the fs-perms.txt file.  I started down this path,
and when the file reached a few hundred lines I gave up and went to the root
cause.. mismatched umask.)

(The global directories are the standard GNU ones, specified in bitbake.conf,
and a hand full of specific documentation and similar...  I could have put these
into the fs-perms.txt, but keeping them dynamic ensures that if someone changes
the value of "libdir", we don't have to specify anything for it to work properly.)

The umask fixes both the directories we should know about, and those we'll never
know about -- recipes that don't exist in oe-core/meta-oe...  It also has a side
effect of ensuring that the recipe author only has to pay attention to basic
recipe tasks, and not do a full world build and look for conflcts.

A good example of the problem is that cups generates a directory /etc/cups and
sets the perms to 0755 w/ uid/gid of root:lp.  When ghostscript comes along to
put something in /etc/cups, it does a "mkdir -p /etc/cups"...  that means
/etc/cups now gets the default mode, uid/gid...  Changing "ghostscript" to match
is easy, it's one package... but what happens when someone adds HP printer
drivers, then epson printer drivers, etc..  you quickly get into a situation
where you really want a global way to affect /etc/cups, and potentially
everything located inside of it.

>> The user can add their own settings, or override the defaults by providing a
>> meta/files/fs-perms.txt file. 
> 
> Using a magically-named file rather than metadata vars is a bit
> unconventional and seems like it will make life unnecessarily harder for
> distros/other overlays.  Is there any reason it needs to be done that
> way?

fs-perms.txt is the default name.  It was done to work similarly to the
'device_table-minimal.txt' file that already exists.

The logic is:

Use the variable FILESYSTEM_PERMS_TABLES, the contents of this may be full
paths, or partial paths within the BBPATH structure.  Multiple files may be
listed space separated.  (Evaluation is is order it's specified, so the last one
loaded takes precedence over earlier ones.)

If the variable FILESYSTEM_PERMS_TABLES is not specified, we then use the
files/fs-perms.txt, again BBPATH is searched for that.

--Mark

> p.
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 19:12   ` Mark Hatle
@ 2011-06-21 21:09     ` Phil Blundell
  2011-06-21 21:27       ` Mark Hatle
  2011-06-21 21:32     ` Koen Kooi
  2011-06-21 22:05     ` Richard Purdie
  2 siblings, 1 reply; 16+ messages in thread
From: Phil Blundell @ 2011-06-21 21:09 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
> fs-perms.txt is the default name.  It was done to work similarly to the
> 'device_table-minimal.txt' file that already exists.
> 
> The logic is:
> 
> Use the variable FILESYSTEM_PERMS_TABLES, the contents of this may be full
> paths, or partial paths within the BBPATH structure.  Multiple files may be
> listed space separated.  (Evaluation is is order it's specified, so the last one
> loaded takes precedence over earlier ones.)
> 
> If the variable FILESYSTEM_PERMS_TABLES is not specified, we then use the
> files/fs-perms.txt, again BBPATH is searched for that.

Okay, I see.  Does it support variable interpolation?  If the
permissions are going to be specified out-of-band then I think at a
minimum there needs to be a way to refer symbolically to things like
${prefix} rather than hardcoding their values.

Also, from what you wrote above about device_table-minimal, it sounds as
though this file is a global manifest rather than being per-recipe.  Is
that right?  If so, it sounds like this is going to be a frequent source
of merge headaches in much the same way as the old checksums.ini was.

p.





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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:09     ` Phil Blundell
@ 2011-06-21 21:27       ` Mark Hatle
  2011-06-21 21:37         ` Phil Blundell
  2011-06-22  5:47         ` Anders Darander
  0 siblings, 2 replies; 16+ messages in thread
From: Mark Hatle @ 2011-06-21 21:27 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

(Note, I found an additional issue mentioned at the end of this email...)

On 6/21/11 4:09 PM, Phil Blundell wrote:
> On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
>> fs-perms.txt is the default name.  It was done to work similarly to the
>> 'device_table-minimal.txt' file that already exists.
>>
>> The logic is:
>>
>> Use the variable FILESYSTEM_PERMS_TABLES, the contents of this may be full
>> paths, or partial paths within the BBPATH structure.  Multiple files may be
>> listed space separated.  (Evaluation is is order it's specified, so the last one
>> loaded takes precedence over earlier ones.)
>>
>> If the variable FILESYSTEM_PERMS_TABLES is not specified, we then use the
>> files/fs-perms.txt, again BBPATH is searched for that.
> 
> Okay, I see.  Does it support variable interpolation?  If the
> permissions are going to be specified out-of-band then I think at a
> minimum there needs to be a way to refer symbolically to things like
> ${prefix} rather than hardcoding their values.

I don't know how to do this within bitbake (easily).  The table is currently a
static set of paths that exist within the distribution as a whole.  They are not
recipe specific, but you can have more then one file... the assumption is
different layers may bring in additional files, only if necessary.

If there is existing code I can use to identify and resolve in-line variables,
I'll be happy to add that into the code.  I'd certainly like to add that
capability, but so far we don't have it.  (Would be nice in the device_table
file.. but I doubt that's practical as the code is not run within python.)

I could certainly write code that parsed each path for "${...}" tear it down and
do a getVar..  But I still suspect there is code elsewhere in bitbake that could
do this for me.

> Also, from what you wrote above about device_table-minimal, it sounds as
> though this file is a global manifest rather than being per-recipe.  Is
> that right?  If so, it sounds like this is going to be a frequent source
> of merge headaches in much the same way as the old checksums.ini was.

Yes, it is global.  I don't see any way around that, as recipe specific items
only affect that recipe.  I started by looking into adding "yet another" set of
variables to the .conf space, but that was shot down as too complicated.

This resulted in only one new variable, a way to define the filesystem, and use
it in all of the recipes automatically.  (One advantage is that if a
distribution decides that /sbin should be 0500, it can make that change globally
without affecting any recipes.)

---

Note, there is one remaining issue that I haven't resolved.  A few directories
are defined in terms of symlinks:

 /var/cache root root 120777 volatile/cache
+/var/cache root root 40755

 /var/log root root 120777 volatile/log
+/var/log root root 40755

 /var/run root root 120777 volatile/run
+/var/run root root 40755

I'm not sure how to resolve this.  If the package(s) that define them as a
directory are installed before the one that tries to make them a symlink into
"volatile" it will could cause a failure -- or at least different system behavior.

I'm tempted to extend the fs_perms file to include a way to specify symlinks..
if one was found then the recipe's install would be modified to mv the directory
to the target of the link, and add in the symlink.

--Mark

> 
> p.
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 19:12   ` Mark Hatle
  2011-06-21 21:09     ` Phil Blundell
@ 2011-06-21 21:32     ` Koen Kooi
  2011-06-21 21:41       ` Mark Hatle
  2011-06-21 22:05     ` Richard Purdie
  2 siblings, 1 reply; 16+ messages in thread
From: Koen Kooi @ 2011-06-21 21:32 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer


Op 21 jun 2011, om 21:12 heeft Mark Hatle het volgende geschreven:

> On 6/21/11 1:57 PM, Phil Blundell wrote:
>> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>> 
>>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
>> 
>> Are you confident that this isn't going to break anything like
>> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
>> to 022 for everything that bitbake does, although I can see that making
>> it be so for particular tasks like do_install() might have some merit.
>> Even in the latter case, though, I wonder whether we should just be
>> paying more attention to recipe hygiene and using "install -m ..." with
>> the permissions that we actually want.
> 
> This is why I bring this up.. I'm a bit concerned that doing it generally will
> have unintended consequences.  So far I am not aware of any.  Moving it to a
> different place in the process may be better.  The only issue I've found so far
> is that just coding int into "do_install" really isn't an option.  Between the
> custom do_install components, various classes, etc.. it's difficult in the
> current infrastructure to find a centralized location to set the value.

Would something like this work?

myclass.bbclass:

addtask mytask before do_package after do_install

mytask() {
	do stuff
}

DISTRO.conf:

# do stuff
INHERIT += "myclass"

regards,

Koen
	


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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:27       ` Mark Hatle
@ 2011-06-21 21:37         ` Phil Blundell
  2011-06-22  0:35           ` Mark Hatle
  2011-06-22  5:47         ` Anders Darander
  1 sibling, 1 reply; 16+ messages in thread
From: Phil Blundell @ 2011-06-21 21:37 UTC (permalink / raw)
  To: Mark Hatle; +Cc: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 16:27 -0500, Mark Hatle wrote:
> I don't know how to do this within bitbake (easily).  The table is currently a
> static set of paths that exist within the distribution as a whole.  They are not
> recipe specific, but you can have more then one file... the assumption is
> different layers may bring in additional files, only if necessary.
> 
> If there is existing code I can use to identify and resolve in-line variables,
> I'll be happy to add that into the code.  I'd certainly like to add that
> capability, but so far we don't have it.  (Would be nice in the device_table
> file.. but I doubt that's practical as the code is not run within python.)

I think bb.data.expand() should do what you need there.

p.





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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:32     ` Koen Kooi
@ 2011-06-21 21:41       ` Mark Hatle
  2011-06-21 21:52         ` Phil Blundell
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Hatle @ 2011-06-21 21:41 UTC (permalink / raw)
  To: openembedded-core

On 6/21/11 4:32 PM, Koen Kooi wrote:
> 
> Op 21 jun 2011, om 21:12 heeft Mark Hatle het volgende geschreven:
> 
>> On 6/21/11 1:57 PM, Phil Blundell wrote:
>>> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>>>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>>>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>>>
>>>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
>>>
>>> Are you confident that this isn't going to break anything like
>>> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
>>> to 022 for everything that bitbake does, although I can see that making
>>> it be so for particular tasks like do_install() might have some merit.
>>> Even in the latter case, though, I wonder whether we should just be
>>> paying more attention to recipe hygiene and using "install -m ..." with
>>> the permissions that we actually want.
>>
>> This is why I bring this up.. I'm a bit concerned that doing it generally will
>> have unintended consequences.  So far I am not aware of any.  Moving it to a
>> different place in the process may be better.  The only issue I've found so far
>> is that just coding int into "do_install" really isn't an option.  Between the
>> custom do_install components, various classes, etc.. it's difficult in the
>> current infrastructure to find a centralized location to set the value.
> 
> Would something like this work?
> 
> myclass.bbclass:
> 
> addtask mytask before do_package after do_install
> 
> mytask() {
> 	do stuff
> }

Unfortunately it won't work as the umask would only be set in the "mytask" -
task.  It needs to be set in all of the do_install and do_package tasks.

The only way to do this (from what Chris L told me) is to setup an event handler
and set the umask when we get into specific events.  I'm not sure if I could
correctly capture do_install and later.. plus if someone did what you are
mentioning, then it would no longer be in the umask 022.

> DISTRO.conf:
> 
> # do stuff
> INHERIT += "myclass"
> 
> regards,
> 
> Koen
> 	
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:41       ` Mark Hatle
@ 2011-06-21 21:52         ` Phil Blundell
  2011-06-21 21:58           ` Phil Blundell
  0 siblings, 1 reply; 16+ messages in thread
From: Phil Blundell @ 2011-06-21 21:52 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 16:41 -0500, Mark Hatle wrote:
> Unfortunately it won't work as the umask would only be set in the "mytask" -
> task.  It needs to be set in all of the do_install and do_package tasks.
> 
> The only way to do this (from what Chris L told me) is to setup an event handler
> and set the umask when we get into specific events.  I'm not sure if I could
> correctly capture do_install and later.. plus if someone did what you are
> mentioning, then it would no longer be in the umask 022.

I think you could probably fix do_install by wrapping it in a python
method.  That is, something along the lines of:

python do_wrapped_install() {
	os.umask(022)
	bb.build.exec_func("install", d)
}
addtask wrapped_install after do_compile

and, obviously, remove the existing addtask for do_install.  You get the
idea, anyway.

For do_package and other tasks which come from classes within oe-core
itself, I continue to think that the right answer is just to fix the
classes to do the right thing.

p.




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:52         ` Phil Blundell
@ 2011-06-21 21:58           ` Phil Blundell
  0 siblings, 0 replies; 16+ messages in thread
From: Phil Blundell @ 2011-06-21 21:58 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 22:52 +0100, Phil Blundell wrote:
> On Tue, 2011-06-21 at 16:41 -0500, Mark Hatle wrote:
> > Unfortunately it won't work as the umask would only be set in the "mytask" -
> > task.  It needs to be set in all of the do_install and do_package tasks.
> > 
> > The only way to do this (from what Chris L told me) is to setup an event handler
> > and set the umask when we get into specific events.  I'm not sure if I could
> > correctly capture do_install and later.. plus if someone did what you are
> > mentioning, then it would no longer be in the umask 022.
> 
> I think you could probably fix do_install by wrapping it in a python
> method.  That is, something along the lines of:

Or, I guess, if you were prepared to countenance patching bitbake, just
set:

do_install[umask] = 022

and arrange for the right things to happen.  That might not be so awful.

p.





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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 19:12   ` Mark Hatle
  2011-06-21 21:09     ` Phil Blundell
  2011-06-21 21:32     ` Koen Kooi
@ 2011-06-21 22:05     ` Richard Purdie
  2011-06-21 22:13       ` Mark Hatle
  2 siblings, 1 reply; 16+ messages in thread
From: Richard Purdie @ 2011-06-21 22:05 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
> On 6/21/11 1:57 PM, Phil Blundell wrote:
> > On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
> >> Adjust the umask to 022.  This resolves the problem of dynamically generated
> >> directories (mkdir -p) and specific files (touch foo) having odd permissions.
> >>
> >> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
> > 
> > Are you confident that this isn't going to break anything like
> > group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
> > to 022 for everything that bitbake does, although I can see that making
> > it be so for particular tasks like do_install() might have some merit.
> > Even in the latter case, though, I wonder whether we should just be
> > paying more attention to recipe hygiene and using "install -m ..." with
> > the permissions that we actually want.
> 
> This is why I bring this up.. I'm a bit concerned that doing it generally will
> have unintended consequences.  So far I am not aware of any.  Moving it to a
> different place in the process may be better.  The only issue I've found so far
> is that just coding int into "do_install" really isn't an option.  Between the
> custom do_install components, various classes, etc.. it's difficult in the
> current infrastructure to find a centralized location to set the value.
> 
> (I'd love to be corrected if someone things of another way of doing it.)  The
> setting of the umask is a very low cost operation, so doing it for certain steps
> shouldn't cause a performance penalty... but until we figure that out this is
> the best and easiest solution I've come up with.

How about a umask flag for tasks?

If bitbake sees it for a given task it would set the umask as indicated
for the task. Cheap and easy and would only impact do_install tasks...

Cheers,

Richard






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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 22:05     ` Richard Purdie
@ 2011-06-21 22:13       ` Mark Hatle
  2011-06-22  4:51         ` Mark Hatle
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Hatle @ 2011-06-21 22:13 UTC (permalink / raw)
  To: openembedded-core

I like that better then trying to wrap do_install and such with special code.

It should be fairly easy to set the default for do_install and do_package then.
 I wonder if there would be a way to "notice" and flag as possible errors tasks
running between do_install and do_package (in a single recipe) that may need the
umask set as well.

--Mark

On 6/21/11 5:05 PM, Richard Purdie wrote:
> On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
>> On 6/21/11 1:57 PM, Phil Blundell wrote:
>>> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>>>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>>>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>>>
>>>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
>>>
>>> Are you confident that this isn't going to break anything like
>>> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
>>> to 022 for everything that bitbake does, although I can see that making
>>> it be so for particular tasks like do_install() might have some merit.
>>> Even in the latter case, though, I wonder whether we should just be
>>> paying more attention to recipe hygiene and using "install -m ..." with
>>> the permissions that we actually want.
>>
>> This is why I bring this up.. I'm a bit concerned that doing it generally will
>> have unintended consequences.  So far I am not aware of any.  Moving it to a
>> different place in the process may be better.  The only issue I've found so far
>> is that just coding int into "do_install" really isn't an option.  Between the
>> custom do_install components, various classes, etc.. it's difficult in the
>> current infrastructure to find a centralized location to set the value.
>>
>> (I'd love to be corrected if someone things of another way of doing it.)  The
>> setting of the umask is a very low cost operation, so doing it for certain steps
>> shouldn't cause a performance penalty... but until we figure that out this is
>> the best and easiest solution I've come up with.
> 
> How about a umask flag for tasks?
> 
> If bitbake sees it for a given task it would set the umask as indicated
> for the task. Cheap and easy and would only impact do_install tasks...
> 
> Cheers,
> 
> Richard
> 
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:37         ` Phil Blundell
@ 2011-06-22  0:35           ` Mark Hatle
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Hatle @ 2011-06-22  0:35 UTC (permalink / raw)
  To: Phil Blundell; +Cc: Patches and discussions about the oe-core layer

On 6/21/11 4:37 PM, Phil Blundell wrote:
> On Tue, 2011-06-21 at 16:27 -0500, Mark Hatle wrote:
>> I don't know how to do this within bitbake (easily).  The table is currently a
>> static set of paths that exist within the distribution as a whole.  They are not
>> recipe specific, but you can have more then one file... the assumption is
>> different layers may bring in additional files, only if necessary.
>>
>> If there is existing code I can use to identify and resolve in-line variables,
>> I'll be happy to add that into the code.  I'd certainly like to add that
>> capability, but so far we don't have it.  (Would be nice in the device_table
>> file.. but I doubt that's practical as the code is not run within python.)
> 
> I think bb.data.expand() should do what you need there.
> 
> p.
> 
> 

Based on the above, I reworked the package.bbclass commit.  The new version is
located at:

http://git.pokylinux.org/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=7b59d7817771ba6bcf85a9816910e2bc036acf52

Functionally it's almost the same, but structurally it has changed a lot.  From
the last version to this version:

* create a local (to fixup_perms) a function to actually setup the data structure

* remove the default documentation, locale and header correction entries
  - move them to the fs-perms.txt

* When processing a "line" to setup the data structure, we expand all of the
variables with it first.  This not only allows for the directory to be set with
a variable, but also the mode, uid, and gid if necessary.

--Mark



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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 22:13       ` Mark Hatle
@ 2011-06-22  4:51         ` Mark Hatle
  2011-06-22 14:04           ` Mark Hatle
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Hatle @ 2011-06-22  4:51 UTC (permalink / raw)
  To: openembedded-core

On 6/21/11 5:13 PM, Mark Hatle wrote:
> I like that better then trying to wrap do_install and such with special code.
> 
> It should be fairly easy to set the default for do_install and do_package then.
>  I wonder if there would be a way to "notice" and flag as possible errors tasks
> running between do_install and do_package (in a single recipe) that may need the
> umask set as well.
> 
> --Mark

I worked out a patch to bitbake for this:

http://git.pokylinux.org/cgit.cgi/poky-contrib/commit/?h=mhatle/bitbake&id=abc25d84b3c0b766bb5d45c0354936eaa4d605c4

The associated changes to oe-core:

Revert of the original umask patch:

http://git.pokylinux.org/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=845ae082627c6a25304e10e72c655e9197f62c01

New patch that enables umask is specific areas:

http://git.pokylinux.org/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=03eb6afb4d1d40ef421085d847cbc42e1795863f

---

Any comments.  I'm not sure I like this task approach, simply because it's more
complicated.  But what I am testing now enables umask of 022 in:

do_install
do_package
do_rootfs
rootfs_<pkg>_do_rootfs
do_populate_sysroot
adt-installer_1.0.bb: do_deploy
linux-tools.inc: do_install_perf

I think that covers everywhere it will be needed...

--Mark

> On 6/21/11 5:05 PM, Richard Purdie wrote:
>> On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
>>> On 6/21/11 1:57 PM, Phil Blundell wrote:
>>>> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>>>>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>>>>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>>>>
>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
>>>>
>>>> Are you confident that this isn't going to break anything like
>>>> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
>>>> to 022 for everything that bitbake does, although I can see that making
>>>> it be so for particular tasks like do_install() might have some merit.
>>>> Even in the latter case, though, I wonder whether we should just be
>>>> paying more attention to recipe hygiene and using "install -m ..." with
>>>> the permissions that we actually want.
>>>
>>> This is why I bring this up.. I'm a bit concerned that doing it generally will
>>> have unintended consequences.  So far I am not aware of any.  Moving it to a
>>> different place in the process may be better.  The only issue I've found so far
>>> is that just coding int into "do_install" really isn't an option.  Between the
>>> custom do_install components, various classes, etc.. it's difficult in the
>>> current infrastructure to find a centralized location to set the value.
>>>
>>> (I'd love to be corrected if someone things of another way of doing it.)  The
>>> setting of the umask is a very low cost operation, so doing it for certain steps
>>> shouldn't cause a performance penalty... but until we figure that out this is
>>> the best and easiest solution I've come up with.
>>
>> How about a umask flag for tasks?
>>
>> If bitbake sees it for a given task it would set the umask as indicated
>> for the task. Cheap and easy and would only impact do_install tasks...
>>
>> Cheers,
>>
>> Richard
>>
>>
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

* Re: Directory permissions and ownership -- RFC
  2011-06-21 21:27       ` Mark Hatle
  2011-06-21 21:37         ` Phil Blundell
@ 2011-06-22  5:47         ` Anders Darander
  1 sibling, 0 replies; 16+ messages in thread
From: Anders Darander @ 2011-06-22  5:47 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Tue, Jun 21, 2011 at 23:27, Mark Hatle <mark.hatle@windriver.com> wrote:
> Note, there is one remaining issue that I haven't resolved.  A few directories
> are defined in terms of symlinks:
>
>  /var/cache root root 120777 volatile/cache
> +/var/cache root root 40755
>
>  /var/log root root 120777 volatile/log
> +/var/log root root 40755
>
>  /var/run root root 120777 volatile/run
> +/var/run root root 40755
>
> I'm not sure how to resolve this.  If the package(s) that define them as a
> directory are installed before the one that tries to make them a symlink into
> "volatile" it will could cause a failure -- or at least different system behavior.
>
> I'm tempted to extend the fs_perms file to include a way to specify symlinks..
> if one was found then the recipe's install would be modified to mv the directory
> to the target of the link, and add in the symlink.

Yes, please try to find a way to extend the fs_perms file to include
handling of
symlinks. Otherwise we are bound to get problems immediately...

The outline of the symlink handling above seems to be fine, assuming you find
a good place to hook it up to in the tasks.

/Anders



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

* Re: Directory permissions and ownership -- RFC
  2011-06-22  4:51         ` Mark Hatle
@ 2011-06-22 14:04           ` Mark Hatle
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Hatle @ 2011-06-22 14:04 UTC (permalink / raw)
  To: openembedded-core

On 6/21/11 11:51 PM, Mark Hatle wrote:
> On 6/21/11 5:13 PM, Mark Hatle wrote:

...

> Any comments.  I'm not sure I like this task approach, simply because it's more
> complicated.  But what I am testing now enables umask of 022 in:
> 
> do_install
> do_package
> do_rootfs
> rootfs_<pkg>_do_rootfs
> do_populate_sysroot
> adt-installer_1.0.bb: do_deploy
> linux-tools.inc: do_install_perf
> 
> I think that covers everywhere it will be needed...

Results from my over night build.

Just setting do_install and do_package isn't enough.  A lot of packages appear
to unpack, patch, configure, compile, and then in do_install copy, while
preserving permissions, from the build directory.  This leads to a number of
files with 0664 and directories with 0775 permissions (when the users umask is 002.)

I can certainly expand this to do_configure and do_compile.  But I'm really
concerned this will quickly grow out of control and end up being very complex...
 (I will attempt to do this and see if I get better results.)

--Mark

> --Mark
> 
>> On 6/21/11 5:05 PM, Richard Purdie wrote:
>>> On Tue, 2011-06-21 at 14:12 -0500, Mark Hatle wrote:
>>>> On 6/21/11 1:57 PM, Phil Blundell wrote:
>>>>> On Tue, 2011-06-21 at 11:43 -0500, Mark Hatle wrote:
>>>>>> Adjust the umask to 022.  This resolves the problem of dynamically generated
>>>>>> directories (mkdir -p) and specific files (touch foo) having odd permissions.
>>>>>>
>>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mhatle/perms&id=d8470b6a8efdbba04cef5d4dc1ce12720fe83621
>>>>>
>>>>> Are you confident that this isn't going to break anything like
>>>>> group-shared DL_DIRs?  I'm not entirely thrilled about forcing the umask
>>>>> to 022 for everything that bitbake does, although I can see that making
>>>>> it be so for particular tasks like do_install() might have some merit.
>>>>> Even in the latter case, though, I wonder whether we should just be
>>>>> paying more attention to recipe hygiene and using "install -m ..." with
>>>>> the permissions that we actually want.
>>>>
>>>> This is why I bring this up.. I'm a bit concerned that doing it generally will
>>>> have unintended consequences.  So far I am not aware of any.  Moving it to a
>>>> different place in the process may be better.  The only issue I've found so far
>>>> is that just coding int into "do_install" really isn't an option.  Between the
>>>> custom do_install components, various classes, etc.. it's difficult in the
>>>> current infrastructure to find a centralized location to set the value.
>>>>
>>>> (I'd love to be corrected if someone things of another way of doing it.)  The
>>>> setting of the umask is a very low cost operation, so doing it for certain steps
>>>> shouldn't cause a performance penalty... but until we figure that out this is
>>>> the best and easiest solution I've come up with.
>>>
>>> How about a umask flag for tasks?
>>>
>>> If bitbake sees it for a given task it would set the umask as indicated
>>> for the task. Cheap and easy and would only impact do_install tasks...
>>>
>>> Cheers,
>>>
>>> Richard
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Openembedded-core mailing list
>>> Openembedded-core@lists.openembedded.org
>>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core




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

end of thread, other threads:[~2011-06-22 18:52 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-21 16:43 Directory permissions and ownership -- RFC Mark Hatle
2011-06-21 18:57 ` Phil Blundell
2011-06-21 19:12   ` Mark Hatle
2011-06-21 21:09     ` Phil Blundell
2011-06-21 21:27       ` Mark Hatle
2011-06-21 21:37         ` Phil Blundell
2011-06-22  0:35           ` Mark Hatle
2011-06-22  5:47         ` Anders Darander
2011-06-21 21:32     ` Koen Kooi
2011-06-21 21:41       ` Mark Hatle
2011-06-21 21:52         ` Phil Blundell
2011-06-21 21:58           ` Phil Blundell
2011-06-21 22:05     ` Richard Purdie
2011-06-21 22:13       ` Mark Hatle
2011-06-22  4:51         ` Mark Hatle
2011-06-22 14:04           ` Mark Hatle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.