netdev - Re: [PATCH v3] selftests: Fix seccomp to support relocatable build (O=objdir)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h7yldohs.fsf@mpe.ellerman.id.au>
Date:   Thu, 19 Mar 2020 14:15:11 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Kees Cook <keescook@...omium.org>
Cc:     Shuah Khan <skhan@...uxfoundation.org>, shuah@...nel.org,
        luto@...capital.net, wad@...omium.org, daniel@...earbox.net,
        kafai@...com, yhs@...com, andriin@...com,
        gregkh@...uxfoundation.org, tglx@...utronix.de,
        khilman@...libre.com, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH v3] selftests: Fix seccomp to support relocatable build (O=objdir)

Kees Cook <keescook@...omium.org> writes:
> On Mon, Mar 16, 2020 at 11:12:57PM +1100, Michael Ellerman wrote:
>> Shuah Khan <skhan@...uxfoundation.org> writes:
>> > Fix seccomp relocatable builds. This is a simple fix to use the right
>> > lib.mk variable TEST_GEN_PROGS with dependency on kselftest_harness.h
>> > header, and defining LDFLAGS for pthread lib.
>> >
>> > Removes custom clean rule which is no longer necessary with the use of
>> > TEST_GEN_PROGS. 
>> >
>> > Uses $(OUTPUT) defined in lib.mk to handle build relocation.
>> >
>> > The following use-cases work with this change:
>> >
>> > In seccomp directory:
>> > make all and make clean
>> >
>> > From top level from main Makefile:
>> > make kselftest-install O=objdir ARCH=arm64 HOSTCC=gcc \
>> >  CROSS_COMPILE=aarch64-linux-gnu- TARGETS=seccomp
>> >
>> > Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
>> > ---
>> >
>> > Changes since v2:
>> > -- Using TEST_GEN_PROGS is sufficient to generate objects.
>> >    Addresses review comments from Kees Cook.
>> >
>> >  tools/testing/selftests/seccomp/Makefile | 18 ++++++++----------
>> >  1 file changed, 8 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile
>> > index 1760b3e39730..a0388fd2c3f2 100644
>> > --- a/tools/testing/selftests/seccomp/Makefile
>> > +++ b/tools/testing/selftests/seccomp/Makefile
>> > @@ -1,17 +1,15 @@
>> >  # SPDX-License-Identifier: GPL-2.0
>> > -all:
>> > -
>> > -include ../lib.mk
>> > +CFLAGS += -Wl,-no-as-needed -Wall
>> > +LDFLAGS += -lpthread
>> >  
>> >  .PHONY: all clean
>> >  
>> > -BINARIES := seccomp_bpf seccomp_benchmark
>> > -CFLAGS += -Wl,-no-as-needed -Wall
>> > +include ../lib.mk
>> > +
>> > +# OUTPUT set by lib.mk
>> > +TEST_GEN_PROGS := $(OUTPUT)/seccomp_bpf $(OUTPUT)/seccomp_benchmark
>> >  
>> > -seccomp_bpf: seccomp_bpf.c ../kselftest_harness.h
>> > -	$(CC) $(CFLAGS) $(LDFLAGS) $< -lpthread -o $@
>> > +$(TEST_GEN_PROGS): ../kselftest_harness.h
>> >  
>> > -TEST_PROGS += $(BINARIES)
>> > -EXTRA_CLEAN := $(BINARIES)
>> > +all: $(TEST_GEN_PROGS)
>> >  
>> > -all: $(BINARIES)
>> 
>> 
>> It shouldn't be that complicated. We just need to define TEST_GEN_PROGS
>> before including lib.mk, and then add the dependency on the harness
>> after we include lib.mk (so that TEST_GEN_PROGS has been updated to
>> prefix $(OUTPUT)).
>> 
>> eg:
>> 
>>   # SPDX-License-Identifier: GPL-2.0
>>   CFLAGS += -Wl,-no-as-needed -Wall
>>   LDFLAGS += -lpthread
>>   
>>   TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark
>>   
>>   include ../lib.mk
>>   
>>   $(TEST_GEN_PROGS): ../kselftest_harness.h
>
> Exactly. This (with an extra comment) is precisely what I suggested during
> v2 review:
> https://lore.kernel.org/lkml/202003041815.B8C73DEC@keescook/

Oh sorry, I missed that.

OK so I think we know what the right solution is.

cheers