On 05/12/2023 00:24, Beata Michalska wrote:
On Tue, Nov 14, 2023 at 04:19:01PM +0000, Zachary Leaf wrote:
Check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall
The aim here should be to verify that the syscall, for a given setup, is being properly handled, not kernel's internals per se - the CHECK_ATTR macro is a helper one and it may disappear at any point of time*, whether be renamed, substituted or removed. In other words - test should be validating syscalls.
Okay. It's true. We're not checking the particular implementation, but we're checking that sending junk in the input fails. So how about just re-word to something like:
The bpf syscall should fail when there is non-zero memory in the bpf_attr input union beyond the last element of the active sub-command struct.
should fail when we have non-zero memory in the union beyond the last element of the active sub-command struct.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
.../kernel/syscalls/bpf/bpf_check_attr.c | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 testcases/kernel/syscalls/bpf/bpf_check_attr.c
diff --git a/testcases/kernel/syscalls/bpf/bpf_check_attr.c b/testcases/kernel/syscalls/bpf/bpf_check_attr.c new file mode 100644 index 000000000..2ad70ee2e --- /dev/null +++ b/testcases/kernel/syscalls/bpf/bpf_check_attr.c
Any particular reason why not to follow the general test naming scheme here ?
I just thought to avoid clashes in future if someone else adds bpf_prog08.c to save you the rebase :)
I can see we did just add epoll_wait{n+1} for what looks to be the only other test we've added as far as I can tell (git log --name-status --diff-filter=A).
Up to you - I can rename it to bpf_prog08 or something else.
@@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) Arm Ltd. 2023. All rights reserved.
- Author: Zachary Leaf zachary.leaf@arm.com
- Check kernel/bpf/syscall.c:CHECK_ATTR macro implementation. The syscall
- should fail when we have non-zero memory in the union beyond the last
- element of the active sub-command struct.
- */
+#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <stddef.h>
+#include "config.h" +#include "tst_test.h" +#include "lapi/bpf.h" +#include "bpf_common.h"
+static char *bpf_log_buf; +static union bpf_attr *attr;
+void run(void) +{
- size_t offset;
- char *ptr;
- printf("sizeof(bpf_attr): %lu\n", sizeof(union bpf_attr));
This looks more like debug log so it might be better to drop this one and the one below (and related includes)
Ack
- memset(attr, 0, sizeof(*attr));
- attr->map_type = BPF_MAP_TYPE_ARRAY;
- attr->key_size = 4;
- attr->value_size = 8;
- attr->max_entries = 1;
- attr->map_flags = 0;
- /*
* test CHECK_ATTR() macro* check syscall fails if there is non-null data somewhere beyond* the last struct member for the BPF_MAP_CREATE option*/- offset = offsetof(union bpf_attr, map_extra);
- printf("offset map_extra: %#lx\n", offset);
- offset += 8;
Nit: this could be handled like kernel's offsetofend.
Good idea, will steal that macro from the kernel.
Thanks, Zach
BR. Beata
- ptr = (char *)attr;
- *(ptr+offset) = 'x';
- TST_EXP_FAIL(bpf(BPF_MAP_CREATE, attr, sizeof(*attr)), EINVAL);
- /* remove the non-null data and BPF_MAP_CREATE should pass */
- *(ptr+offset) = '\0';
- TST_EXP_POSITIVE(bpf_map_create(attr));
+}
+static struct tst_test test = {
- .test_all = run,
- .min_kver = "5.16", /* map_extra field added in commit 9330986c0300 */
- .bufs = (struct tst_buffers []) {
{&bpf_log_buf, .size = BUFSIZE},{&attr, .size = sizeof(*attr)},{},- }
+};
2.34.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org