[Toybox] Integration of SMACK

José Bollo jobol at nonadev.net
Thu Apr 30 05:49:49 PDT 2015


Le jeudi 30 avril 2015 à 00:15 -0500, Rob Landley a écrit :
> 
> I mentioned the redundant path creation and switching it out for openat(O_PATH)
> and fgetxattr(). (If smack vetoes this open in ls, smack is probably going to
> have problems with other programs using modern apis.)

Hello Rob,

Thanks a lot for all of your patience and good work. I checked your
patch (despite that I didn't seen the starting point). That is great.
Bravo.

Of course I changed things but very little.

There is however something bad. I wanted to use O_NOATIME but found that
it is not defined. The issue can be solved by defining _GNU_SOURCE at
the beginning of ls.c Halas it fails because of basename redefinition.
So I added the defining of O_NOATIME in portability.


> The first -Z hunk used lgetxattr() but the second used getxattr(). So we
> measured the length of the symlink attribute, then printed the file attribute?
> I'm consistently using openat(O_PATH|O_NOFOLLOW) which should give me a
> filehandle to the symlink...
> 
> We're testing whether getxattr() returns a length > SMACK_LABEL_LEN. If that
> can happen, did it stomp stack data outside the buffer? If it can't happen,
> why are we testing for it?

Ooops that is my fault. IIRC, I saw that getxattr was used where
lgetxattr is to be used and began to change it. But I finished not. And
in fact the behaviour has to follow the -L option. But doing that is
producing unexpected result that I should investigate.

> Query: we feed sizeof(zlen) to getxattr(), I.E. SMACK_LABEL_LEN+1. But we
> write a nul terminator, so the getxattr() call isn't terminating the buffer,
> so why are we feeding it a buffer size _including_ space for a null terminator?
> 
> The third -Z hunk is identical to the second -Z hunk except it's left
> justified instead of right justified. Also your length is zlen-1 in hunk 3
> and zlen without the -1 in hunk 2? Let's see... what you want is always one
> space on the right, only a space on the left for -long, and yes the left/right
> justification changes between -C and -l for no apparent reason but that's
> what you expect so... (Note: sprintf %*s will left justify if fed a negative
> length. Posix 2008 says so.)

Yes true. We are all expecting that "if (CFG_LS_SMACK)" fails then the
code is removed by the compiler. So creating a function that is never
called is ... Well it's ok

> Why are you typecasting zlen to (int)? Doesn't subtracting a ssize_t work?

On 64 bits arch, ssize_t is 64 bits and int is 32 bits. Does it explain?

> The query is setting a length of 0 if getxattr() doesn't find any data,
> but you're printing "?" and a space, meaning the column sizing logic circa
> line 550 needs adjusting too. (totpad += totals[7]+!!totals[7])

>From what I sew of your new implementation, the way how the spaces
between fields is counted is mysterious. From my tests, it is not
working because it is not counted.

> Alright, here's a wild guess at a patch, which I can't test because
> the tizen image I installed following the instructions on
> https://wiki.tizen.org/wiki/Tizen_Standalone_Emulator hasn't got a native
> toolchain.

(snip)

I'm working on your proposal and will soon produce a submission. The
current difficulty is that calling "dirtree_parentfd(dt)" is currently
not reliable. I'm still working on... Result in few minutes.

Best regards
José

> Rob
> 
> P.S. I note that pops up a useless gui which is taller than my netbook's
> screen. Although said gui lets you launch a terminal window with a really
> tiny font, it completely ignores the keyboard so you can't _type_ anything
> into it. However, if you edit run.sh to change -serial to point to /dev/tty,
> then you get a console on stdin/stdout of the qemu process. Well, your bespoke
> forked qemu, anyway. At least it lets me see what "ls -Z /" and "ls -lZ /" are
> supposed to output...
> 
> P.P.S. Ah, I see that if you right click you can switch the keyboard on and
> change the scale, but at 1/4 size the font in the terminal is _hilariously_
> tiny (my vision isn't that great) so I'm sticking with the serial console
> on stdio thing. Luckily google found I can login as "root/tizen".
> 
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net



 1430398189.0


More information about the Toybox mailing list