Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the new introduced prog_attach_flags are not compatible #41

Closed
mrpre opened this issue Oct 10, 2022 · 2 comments
Closed

the new introduced prog_attach_flags are not compatible #41

mrpre opened this issue Oct 10, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@mrpre
Copy link

mrpre commented Oct 10, 2022

bpftool/src/cgroup.c

Lines 235 to 245 in 43b5daa

__u32 iter;
int ret;
p.query_flags = query_flags;
p.prog_cnt = ARRAY_SIZE(prog_ids);
p.prog_ids = prog_ids;
p.prog_attach_flags = prog_attach_flags;
ret = bpf_prog_query_opts(cgroup_fd, type, &p);
if (ret)
return ret;

prog_attach_flags is the latest field in uapi but not exist in older kernel
https://github.com/torvalds/linux/blob/7d2a07b769330c34b4deabeed939325c77a7ec2f/include/uapi/linux/bpf.h#L1400-L1407
and in kernel it will ensure all fields after it's last field(prog_cnt) must be zero

/* helper macro to check that unused fields 'union bpf_attr' are zero */
#define CHECK_ATTR(CMD) \
	memchr_inv((void *) &attr->CMD##_LAST_FIELD + \
		   sizeof(attr->CMD##_LAST_FIELD), 0, \
		   sizeof(*attr) - \
		   offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
		   sizeof(attr->CMD##_LAST_FIELD)) != NULL

#define BPF_PROG_QUERY_LAST_FIELD query.prog_cnt

static int bpf_prog_query(const union bpf_attr *attr,
			  union bpf_attr __user *uattr)
{
	if (!capable(CAP_NET_ADMIN))
		return -EPERM;
	if (CHECK_ATTR(BPF_PROG_QUERY))
		return -EINVAL;
         ...
}  

It seems the upstream kernel doesn't have the compatibility issue because libbpf/bpftool is as the part of it's source.

@qmonnet
Copy link
Member

qmonnet commented Oct 10, 2022

Thanks for the report! So what happens in that case? It breaks bpftool cgroup tree, is this correct?

Relevant upstream commits: kernel, bpftool (Linux 6.1). I suppose the fix will be to retry if we get -EINVAL at this step (assuming that's what you get from the syscall?).

@qmonnet qmonnet added the bug Something isn't working label Jan 8, 2023
@qmonnet
Copy link
Member

qmonnet commented Oct 29, 2024

No feedback and no other report in two years, I'm closing the issue.

@qmonnet qmonnet closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants