On Mon, Jan 11, 2016 at 1:17 PM, Rob Landley <[email protected]> wrote:
> On 01/09/2016 03:38 PM, enh wrote:
>> i have the following internal bug:
>>
>> ls -laZ doesn't properly return the SELinux label for files where a
>> non-fully qualified path is provided.
>>
>> Steps to reproduce (on Nexus 9):
>>
>>   adb shell ls -laZ /sys/kernel/debug/tracing/tracing_on
>>   adb shell ls -laZ /sys/kernel/debug/tracing/ | grep tracing_on
>>   adb shell 'cd /sys/kernel/debug/tracing ; ls -laZ tracing_on'
>>
>> Expected: The output of all commands should yield the same SELinux
>> label: "u:object_r:debugfs_tracing:s0"
>
> I have now ordered a 1 gig quad processor raspberry pi "b" to try to
> fling marshmallow upon. (I really need an android test environment, and
> that's powerful enough to potentially self-host, so...)
>
> (I would have ordered a b+ but those are all 512M with 700 mhz processor
> and I think it's only dual core... Not sure how "+" works into it here.)

(i realize you probably just wanted an excuse to buy a Raspberry Pi,
but the x86 emulator using kvm is very fast, you know.)

>> something like this seems sensible for ls:
>>
>> diff --git a/toys/posix/ls.c b/toys/posix/ls.c
>> index 2f44d24..08ae695 100644
>> --- a/toys/posix/ls.c
>> +++ b/toys/posix/ls.c
>> @@ -541,6 +541,7 @@ void ls_main(void)
>>    // Iterate through command line arguments, collecting directories and 
>> files.
>>    // Non-absolute paths are relative to current directory.
>>    TT.files = dirtree_start(0, 0);
>> +  TT.files->dirfd = AT_FDCWD;
>>    for (s = *toys.optargs ? toys.optargs : noargs; *s; s++) {
>>      dt = dirtree_start(*s, !(toys.optflags&(FLAG_l|FLAG_d|FLAG_F)) ||
>>                              (toys.optflags&(FLAG_L|FLAG_H)));
>
> Yes, that's the correct fix. (I just independently did that before
> noticing the second half of your message. :)

not sure whether you mean you've already applied a fix but not pushed,
but in case you haven't, patch attached.

>> but i'm not sure whether the semantics of dirtree_start are supposed
>> to imply this? certainly defaulting dirfd to -1 rather than 0 might
>> lead to fewer surprises in future.
>
> The ls recursion is unusual because ls's recursion semantics are funky.
> (It does two passes over the data to pad out columns, the "label this
> directory or not" logic involves knowledge of other arguments, and so on.)
>
> So to implement what posix wants, ls takes manual control over recursion
> (calling dirtree_recurse() itself in listfiles()), which uses mostly the
> same data structure and mostly the same functions, but just... connects
> them up strangely so it can do crazy things that straddle nodes.
>
> This is why there's a fake node at the top of the ls tree representing
> ".", which is not usually the case. (The manual recursion in ls requires
> a parent node to operate, the common infrastructure doesn't). So our
> "did we fall off the top of the tree logic and have no ->parent,
> therefore AT_CWD" logic doesn't trigger, and sticking AT_CWD into the
> fake top of tree node as its dirfd is the correct response.
>
> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
From ef47be261ecf7f775f5d9de0f10812d1f51553c8 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <[email protected]>
Date: Tue, 12 Jan 2016 12:49:58 -0800
Subject: [PATCH] Fix ls -Z with relative paths.

ls -Z uses dirtree::dirfd to read security attributes via the O_PATH hack.
This is fine for absolute paths because the fd is ignored, but for relative
paths we need AT_FDCWD.

The ls recursion is unusual because ls's recursion semantics are funky.
(It does two passes over the data to pad out columns, the "label this
directory or not" logic involves knowledge of other arguments, and so on.)

So to implement what posix wants, ls takes manual control over recursion
(calling dirtree_recurse() itself in listfiles()), which uses mostly the
same data structure and mostly the same functions, but just... connects
them up strangely so it can do crazy things that straddle nodes.

This is why there's a fake node at the top of the ls tree representing
".", which is not usually the case. (The manual recursion in ls requires
a parent node to operate, the common infrastructure doesn't). So our
"did we fall off the top of the tree logic and have no ->parent,
therefore AT_CWD" logic doesn't trigger, and sticking AT_CWD into the
fake top of tree node as its dirfd is the correct response.
---
 toys/posix/ls.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/toys/posix/ls.c b/toys/posix/ls.c
index 2f44d24..08ae695 100644
--- a/toys/posix/ls.c
+++ b/toys/posix/ls.c
@@ -541,6 +541,7 @@ void ls_main(void)
   // Iterate through command line arguments, collecting directories and files.
   // Non-absolute paths are relative to current directory.
   TT.files = dirtree_start(0, 0);
+  TT.files->dirfd = AT_FDCWD;
   for (s = *toys.optargs ? toys.optargs : noargs; *s; s++) {
     dt = dirtree_start(*s, !(toys.optflags&(FLAG_l|FLAG_d|FLAG_F)) ||
                             (toys.optflags&(FLAG_L|FLAG_H)));
-- 
2.6.0.rc2.230.g3dd15c0

_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to