On Tue, Jan 21, 2025 at 04:41:17PM +0100, hsimeliere.opensource@xxxxxxxxxxx wrote: > From: Xu Kuohai <xukuohai@xxxxxxxxxx> > > [ Upstream commit 28ead3eaabc16ecc907cfb71876da028080f6356 ] > > bpf progs can be attached to kernel functions, and the attached functions > can take different parameters or return different return values. If > prog attached to one kernel function tail calls prog attached to another > kernel function, the ctx access or return value verification could be > bypassed. > > For example, if prog1 is attached to func1 which takes only 1 parameter > and prog2 is attached to func2 which takes two parameters. Since verifier > assumes the bpf ctx passed to prog2 is constructed based on func2's > prototype, verifier allows prog2 to access the second parameter from > the bpf ctx passed to it. The problem is that verifier does not prevent > prog1 from passing its bpf ctx to prog2 via tail call. In this case, > the bpf ctx passed to prog2 is constructed from func1 instead of func2, > that is, the assumption for ctx access verification is bypassed. > > Another example, if BPF LSM prog1 is attached to hook file_alloc_security, > and BPF LSM prog2 is attached to hook bpf_lsm_audit_rule_known. Verifier > knows the return value rules for these two hooks, e.g. it is legal for > bpf_lsm_audit_rule_known to return positive number 1, and it is illegal > for file_alloc_security to return positive number. So verifier allows > prog2 to return positive number 1, but does not allow prog1 to return > positive number. The problem is that verifier does not prevent prog1 > from calling prog2 via tail call. In this case, prog2's return value 1 > will be used as the return value for prog1's hook file_alloc_security. > That is, the return value rule is bypassed. > > This patch adds restriction for tail call to prevent such bypasses. > > Signed-off-by: Xu Kuohai <xukuohai@xxxxxxxxxx> > Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@xxxxxxxxxxxxxxx > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > [ Deletion of the patch line on the condition using bpf_prog_is_dev_bound as > it was added by commit 3d76a4d3d4e591af3e789698affaad88a5a8e8ab which is not > present in version 6.1 ] > Signed-off-by: BRUNO VERNAY <bruno.vernay@xxxxxx> > Signed-off-by: Hugo SIMELIERE <hsimeliere.opensource@xxxxxxxxxxx> > --- > include/linux/bpf.h | 1 + > kernel/bpf/core.c | 19 +++++++++++++++++-- > 2 files changed, 18 insertions(+), 2 deletions(-) Why is this commit needed in the 6.1.y kernel tree? confused, greg k-h
