Re: [PATCH net-next v6 10/11] bpf,landlock: Add tests for Landlock

[Kees Cook]

On Tue, Apr 18, 2017 at 4:53 PM, MickaÃl SalaÃn <mic@xxxxxxxxxxx> wrote:
> On 19/04/2017 01:16, Kees Cook wrote:
>> On Tue, Mar 28, 2017 at 4:46 PM, MickaÃl SalaÃn <mic@xxxxxxxxxxx> wrote:
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/landlock/Makefile
>>> @@ -0,0 +1,47 @@
>>> +LIBDIR := ../../../lib
>>> +BPFOBJ := $(LIBDIR)/bpf/bpf.o
>>> +LOADOBJ := ../../../../samples/bpf/bpf_load.o
>>
>> Is the selftest tarball creation tool okay with this? IIRC, it should
>> be fine since it'll be a built object already, but it's a random
>> thought I had while looking at this.
>
> Hum, I'll check since it's the same for BPF tests.

Okay, cool.

>>> +# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>>> +# But, there is no easy way to fix it, so just exclude it since it is
>>> +# useless for BPF samples.
>>> +$(obj)/%.o: $(src)/%.c
>>> +       $(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>>> +               -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>>> +               -Wno-compare-distinct-pointer-types \
>>> +               -Wno-gnu-variable-sized-type-not-at-end \
>>> +               -Wno-tautological-compare \
>>> +               -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
>>
>> Is clang required for the samples and the selftests? That needs to be
>> avoided... there needs to be a way to show people how to build a
>> landlock rule without requiring clang.
>
> I can rewrite this tests without requiring clang but it is already
> required for BPF testsâ

So, I guess it's not a big deal for selftests (but it'd be nice, even
for BPF), but I think at least the samples/ should have examples on
how to do it "by hand", etc. Not everyone will build stuff with clang,
and it'd be good to make landlock as available as possible.

>>> +#define ASSERT_STEP(cond) \
>>> +       { \
>>> +               step--; \
>>> +               if (!(cond)) \
>>> +                       _exit(step); \
>>> +       }
>>
>> Can you explain this in more detail? I'm assuming there is a problem
>> with writing to the TH_LOG_STREAM fd or something?
>
> It's a trick to use the test framework without requiring to be allowed
> to write to an FD (i.e. log stream), but only to exit a code. I use this
> to test a Landlock rule which forbid access to any FS objects (including
> open FD). This could be used for seccomp too.

Okay. For seccomp, we just allow the fd. :P I'm not opposed to it; it
just makes some debugging harder without text details, etc.

-Kees

-- 
Kees Cook
Pixel Security