[Toybox] Integration of SMACK

[Rob Landley]

On 04/22/2015 11:45 AM, José Bollo wrote:
> Le samedi 18 avril 2015 à 09:37 -0500, Rob Landley a écrit :
> Jan is correct, I tried the following command "ls -is" and the result is
> not well formatted because the size of the numbers are not well
> computed. And the count of collum is also badly computed as the space
> between columns are not taken into account, see companion GIF image.

When I got the reproduction sequence (which I see now was in the summary
comment at the top of the broken-out version of the patch), I fixed it.

>> I tried applying your ls patch without this, but got blocked on the
>> fact your patch modifies bits of the previous patch (in at least one
>> case undoing a change the earlier patch made), so there are multiple
>> failed hunks.
>>
>> Meanwhile, your ls patch fetches and then frees the same absolute path
>> three times, which is redundant. It queries data using a potentially
>> long path, which is racy and inefficient, and doesn't match the rest
>> of the dirtree stuff which uses openat() and friends.
> 
> After mining the question, the problem can be divided in several parts.
> 
> 1. Why the functions (l)getxattr don't have the (l)getxattr(at) variant
> in there API? Typing "man syscalls" (with a tailing s) and searching in
> that manual shows that it doesn't exist event at the kernel ABI level.
> Thus it can not be used.

There's an fgetattr() which prsumably can be made to work, if I can open
a filehandle that's not in read

> 2. Why the API of dirtree doesn't have a scratch buffer of at most
> PATH_MAX to provide the same behaviour? Well it would be easier than
> changing ABI of the kernel. Having a scratch buffer might be easy. The
> same function, dirtree_path, could return always the same scratch
> buffer. If you need it for a long period or because of it can be
> overwritten, just strdup it and later free it.

The reason it doesn't is PATH_MAX went away years ago. It never made
sense: you can "mkdir -p one/two/three/four/five", cd into that, and
them "mv ~/equally/long/tree .", and thus no matter what your "maximum"
path value is you can always construct one in the filesystem that's
longer, and then the tools just have to cope. (Posix specifically
requires rm to deal with arbitrary depth, which means even my "keep an
open filehandle per level" logic has to go; I have a design to fix that
but don't think I've implemented it yet.)

That's why all the modern apis work based on openat() and friends. The
alternative is constantly changing cwd, which gets racy and evil even
_without_ threads.

That said, I do have two 4k buffers, toybuf and libbuf. The first is
usable by any command and should never be modified by library code, the
second is usable by library code and should never be modified outside of
lib/*.c. But I don't use them for this because it would be wrong for the
above mentioned reasons: it introduces limits to the API that aren't
actually there in the filesystem.

And really, I'm crazy enough to want my dirtree stuff to work if
somebody does a "mv" on one of the directories in the middle while I'm
in the middle of traversing it. Re-traversing the path each time would
go nuts,

>> I don't see a getxattr() variant that lets us feed it a directory
>> descriptor instead of using curdir() (apparently not many kernel
>> developers ever actually use this syscall), but it does at least have
>> fgetxattr() which can presumably be stacked on openat(). Or at least
>> it could in a sane world, but this is security theatre code we're
>> talking about here so "sane" is a big assumption: how do I know it
>> isn't going to veto an O_RDONLY open of the file even before anybody
>> tries to read data from it, so we can't actually get an inactive
>> filehandle to this thing we can use to query extended attributes but
>> not read file contents from...?

In 2.6.39 the kernel guys added the O_PATH flag:

http://man7.org/linux/man-pages/man2/open.2.html

Which allows you to open a file without read permissions. (O_RDONLY is
zero so you can't _not_ specify "I would like read permission on this
file". So they had to add O_PATH to say "switch off the O_RDONLY bit
even though there isn't one".)

The point of this would be to let you open a chmod 000 file via openat()
and then use functions like fgetxattr() on it. (And this eliminates some
races because you can fstat() the filehandle and go "yup, this is a
directory" before proceeding.)

I'm leaning towards rewriting your ls patch to use openat() with O_PATH
and use that to avoid allocating, traversing, and freeing full paths
from cwd for every file.

Of course I dunno if your security infrastructure is going to veto the
open anyway. Still can't test it...

Rob

 1430333885.0