netdev - Re: [PATCH v6 bpf-next 4/6] selftests/bpf: add bpf_snprintf

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: <20200925005051.nqf6ru46psex7oh4@ast-mbp.dhcp.thefacebook.com>
Date:   Thu, 24 Sep 2020 17:50:51 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Alan Maguire <alan.maguire@...cle.com>
Cc:     ast@...nel.org, daniel@...earbox.net, andriin@...com, yhs@...com,
        linux@...musvillemoes.dk, andriy.shevchenko@...ux.intel.com,
        pmladek@...e.com, kafai@...com, songliubraving@...com,
        john.fastabend@...il.com, kpsingh@...omium.org, shuah@...nel.org,
        rdna@...com, scott.branden@...adcom.com, quentin@...valent.com,
        cneirabustos@...il.com, jakub@...udflare.com, mingo@...hat.com,
        rostedt@...dmis.org, bpf@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        acme@...nel.org
Subject: Re: [PATCH v6 bpf-next 4/6] selftests/bpf: add bpf_snprintf_btf
 helper tests

On Wed, Sep 23, 2020 at 06:46:26PM +0100, Alan Maguire wrote:
> +static int __strncmp(const void *m1, const void *m2, size_t len)
> +{
> +	const unsigned char *s1 = m1;
> +	const unsigned char *s2 = m2;
> +	int i, delta = 0;
> +
> +#pragma clang loop unroll(full)

Shouldn't be needed?
The verifier supports bounded loops.

> +	for (i = 0; i < len; i++) {
> +		delta = s1[i] - s2[i];
> +		if (delta || s1[i] == 0 || s2[i] == 0)
> +			break;
> +	}
> +	return delta;
> +}
> +
> +/* Use __builtin_btf_type_id to test snprintf_btf by type id instead of name */
> +#if __has_builtin(__builtin_btf_type_id)
> +#define TEST_BTF_BY_ID(_str, _typestr, _ptr, _hflags)			\
> +	do {								\
> +		int _expected_ret = ret;				\
> +		_ptr.type = 0;						\
> +		_ptr.type_id = __builtin_btf_type_id(_typestr, 0);	\

The test is passing for me, but I don't understand why :)
__builtin_btf_type_id(, 0); means btf_id of the bpf program.
While bpf_snprintf_btf() is treating it as btf_id of vmlinux_btf.
So it really should have been __builtin_btf_type_id(,1);

The following diff works:
diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
index b4f96f1f6830..bffa786e3b03 100644
--- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
+++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
@@ -45,7 +45,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
        do {                                                            \
                int _expected_ret = ret;                                \
                _ptr.type = 0;                                          \
-               _ptr.type_id = __builtin_btf_type_id(_typestr, 0);      \
+               _ptr.type_id = __builtin_btf_type_id(_typestr, 1);      \
                ret = bpf_snprintf_btf(_str, STRSIZE, &_ptr,            \
                                       sizeof(_ptr), _hflags);          \
                if (ret != _expected_ret) {                             \
@@ -88,7 +88,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
                        ret = -EBADMSG;                                 \
                        break;                                          \
                }                                                       \
-               TEST_BTF_BY_ID(_str, #_type, _ptr, _hflags);            \
+               TEST_BTF_BY_ID(_str, _ptr, _ptr, _hflags);              \

But still makes me suspicious of the test. I haven't debugged further.