[Toybox] Integration of SMACK

José Bollo jobol at nonadev.net
Mon Apr 20 00:01:34 PDT 2015


Le samedi 18 avril 2015 à 09:37 -0500, Rob Landley a écrit :
> 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,

Hi,

Great...

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

Well I should check. I'm pushing the work of Jan.

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

I also should check.

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

You are perfectly right.

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

Here again, you are right.

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

You need some appliance that activate Smack. I'll check it.

Unfortunately, today is a day off. So nothing before tomorrow...

Best regards
José

> Rob



 1429513294.0


More information about the Toybox mailing list