[Toybox] Integration of SMACK

Rob Landley rob at landley.net
Sat Apr 18 07:37:59 PDT 2015


On Fri, Apr 17, 2015 at 4:29 AM, José Bollo <jobol at nonadev.net> wrote:
> Hi all,
>
> I've just abandonned my 3 pull requests and created a new one. It
> includes improvements suggested by Rob ibn a private discussion.
>
> Feedbacks are welcome

I merged the first patch in the list, fluffing out the #ifdef/#else
for smack in portability.h, but the second commit is another one of
those "this has nothing to do with smack but you've made it a
prerequisite to all the patches that _are_ about smack" patch.
Specifically:

>From f84019f3c40c4193051e0eb4f3d4cb5313f3f5ed Mon Sep 17 00:00:00 2001
From: Jan Cybulski <j.cybulski at samsung.com>
Date: Fri, 17 Oct 2014 08:59:24 +0200
Subject: [PATCH 2/9] ls: Fix identation of number of blocks and inodes

Identation was not properly done, which was especially seen with -l option.

Change-Id: I681f6f0b8cf105f7f88f0dfc7779718895f9521c
Signed-off-by: Jan Cybulski <j.cybulski at samsung.com>

Define "not properly done". There's no test to show the problem, and
no description of the problem that would let me know what problem the
developer thinks this fixed. I just checked the -l option again and
it's calculating all the column offsets like it always was and they
look fine to me?

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.

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...?

Basically I'm still blocked by the fact I can't _test_ this stuff. I
don't have an environment I can boot up under kvm with a toolchain
with smack headers installed so I can build this and run the result.
So if I make any changes, I can't try the result. (Somebody mentioned
poky? Is there an ISO image I can install under kvm?)

Rob

 1429367879.0


More information about the Toybox mailing list