From: Xu Kuohai xukuohai@huawei.com
[ 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@huawei.com Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: BRUNO VERNAY bruno.vernay@se.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- include/linux/bpf.h | 1 + kernel/bpf/core.c | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 7f4ce183dcb0..39291ec48374 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -250,6 +250,7 @@ struct bpf_map { * same prog type, JITed flag and xdp_has_frags flag. */ struct { + const struct btf_type *attach_func_proto; spinlock_t lock; enum bpf_prog_type type; bool jited; diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 83b416af4da1..c281f5b8705e 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2121,6 +2121,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map, { enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; + struct bpf_prog_aux *aux = fp->aux;
if (fp->kprobe_override) return false; @@ -2132,12 +2133,26 @@ bool bpf_prog_map_compatible(struct bpf_map *map, */ map->owner.type = prog_type; map->owner.jited = fp->jited; - map->owner.xdp_has_frags = fp->aux->xdp_has_frags; + map->owner.xdp_has_frags = aux->xdp_has_frags; + map->owner.attach_func_proto = aux->attach_func_proto; ret = true; } else { ret = map->owner.type == prog_type && map->owner.jited == fp->jited && - map->owner.xdp_has_frags == fp->aux->xdp_has_frags; + map->owner.xdp_has_frags == aux->xdp_has_frags; + if (ret && + map->owner.attach_func_proto != aux->attach_func_proto) { + switch (prog_type) { + case BPF_PROG_TYPE_TRACING: + case BPF_PROG_TYPE_LSM: + case BPF_PROG_TYPE_EXT: + case BPF_PROG_TYPE_STRUCT_OPS: + ret = false; + break; + default: + break; + } + } } spin_unlock(&map->owner.lock);
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 28ead3eaabc16ecc907cfb71876da028080f6356
WARNING: Author mismatch between patch and upstream commit: Backport author: hsimeliere.opensource@witekio.com Commit author: Xu Kuohaixukuohai@huawei.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 5d5e3b4cbe8e) 6.1.y | Not found
Note: The patch differs from the upstream commit: --- 1: 28ead3eaabc1 ! 1: 40e6bff2b282 bpf: Prevent tail call between progs attached to different hooks @@ Metadata ## Commit message ## bpf: Prevent tail call between progs attached to different hooks
+ [ 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 @@ Commit message Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org + Signed-off-by: BRUNO VERNAY bruno.vernay@se.com + Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com
## include/linux/bpf.h ## @@ include/linux/bpf.h: struct bpf_map { @@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map,
if (fp->kprobe_override) return false; -@@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map, - * in the case of devmap and cpumap). Until device checks - * are implemented, prohibit adding dev-bound programs to program maps. - */ -- if (bpf_prog_is_dev_bound(fp->aux)) -+ if (bpf_prog_is_dev_bound(aux)) - return false; - - spin_lock(&map->owner.lock); @@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map, */ map->owner.type = prog_type; ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
On Fri, Jan 10, 2025 at 09:40:00AM +0100, hsimeliere.opensource@witekio.com wrote:
From: Xu Kuohai xukuohai@huawei.com
[ 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@huawei.com Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: BRUNO VERNAY bruno.vernay@se.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com
Please document what you are doing here that is needed for the backport as this does NOT match up with what is upstream (a chunk is missing...)
thanks,
greg k-h
From: Xu Kuohai xukuohai@huawei.com
[ 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@huawei.com Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org [ 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@se.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com --- include/linux/bpf.h | 1 + kernel/bpf/core.c | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 2189c0d18fa7..e9c1338851e3 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -250,6 +250,7 @@ struct bpf_map { * same prog type, JITed flag and xdp_has_frags flag. */ struct { + const struct btf_type *attach_func_proto; spinlock_t lock; enum bpf_prog_type type; bool jited; diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 83b416af4da1..c281f5b8705e 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2121,6 +2121,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map, { enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; + struct bpf_prog_aux *aux = fp->aux;
if (fp->kprobe_override) return false; @@ -2132,12 +2133,26 @@ bool bpf_prog_map_compatible(struct bpf_map *map, */ map->owner.type = prog_type; map->owner.jited = fp->jited; - map->owner.xdp_has_frags = fp->aux->xdp_has_frags; + map->owner.xdp_has_frags = aux->xdp_has_frags; + map->owner.attach_func_proto = aux->attach_func_proto; ret = true; } else { ret = map->owner.type == prog_type && map->owner.jited == fp->jited && - map->owner.xdp_has_frags == fp->aux->xdp_has_frags; + map->owner.xdp_has_frags == aux->xdp_has_frags; + if (ret && + map->owner.attach_func_proto != aux->attach_func_proto) { + switch (prog_type) { + case BPF_PROG_TYPE_TRACING: + case BPF_PROG_TYPE_LSM: + case BPF_PROG_TYPE_EXT: + case BPF_PROG_TYPE_STRUCT_OPS: + ret = false; + break; + default: + break; + } + } } spin_unlock(&map->owner.lock);
On Tue, Jan 21, 2025 at 04:41:17PM +0100, hsimeliere.opensource@witekio.com wrote:
From: Xu Kuohai xukuohai@huawei.com
[ 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@huawei.com Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org [ 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@se.com Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com
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
This patch is needed to correct CVE-2024-50063 https://nvd.nist.gov/vuln/detail/CVE-2024-50063
Kind regards, Hugo Simeliere
On Mon, Feb 10, 2025 at 10:44:07AM +0100, hsimeliere.opensource@witekio.com wrote:
This patch is needed to correct CVE-2024-50063
What patch? I see no context here :(
Never link to nvd, their "enhancements" are provably wrong and hurtful to the kernel ecosystem. Always just refer to cve.org records or better yet, our own announcements.
That being said, why do you feel this commit is really needed in those older kernels other than just trying to meet a "check box" requirement somewhere?
thanks,
greg k-h
Thank you for this information, I will take note of it for our next contribution. Does this also apply to more recent kernel versions?
So the CVE must be under a CNA or CISA score for the patch to be required by the kernel? Where can I find your own announcements?
Thanks, Hugo Simeliere
On Mon, Feb 10, 2025 at 03:29:05PM +0100, hsimeliere.opensource@witekio.com wrote:
Thank you for this information, I will take note of it for our next contribution. Does this also apply to more recent kernel versions?
What information? Sorry, but I see no context here at all, have you read Documentation/process/email-clients.rst
Remember, some of us get 1000+ emails a day to deal with, context matters.
thanks,
greg k-h
On Mon, Feb 10, 2025 at 10:55:07AM +0100, gregkh@linuxfoundation.org wrote:
Never link to nvd, their "enhancements" are provably wrong and hurtful to the kernel ecosystem. Always just refer to cve.org records or better yet, our own announcements.
Thank you for this information, I will take note of it for our next contribution. So the CVE must be under a CNA or CISA score for the patch to be required by the kernel? Where can I find your own announcements?
Thanks, Hugo Simeliere
On Mon, Feb 10, 2025 at 05:32:33PM +0100, hsimeliere.opensource@witekio.com wrote:
On Mon, Feb 10, 2025 at 10:55:07AM +0100, gregkh@linuxfoundation.org wrote:
Never link to nvd, their "enhancements" are provably wrong and hurtful to the kernel ecosystem. Always just refer to cve.org records or better yet, our own announcements.
Thank you for this information, I will take note of it for our next contribution. So the CVE must be under a CNA or CISA score for the patch to be required by the kernel?
The kernel CNA provides NO "score" as that obviously is impossible to do given that we do NOT know your use case.
What exactly are you trying to do here? Backport random changes to older kernels for what reason? We are glad to take backports for fixes that did not apply to older kernels, but you have to test them and provide a reason for why they should be included. To not have that on your side already feels very odd.
Where can I find your own announcements?
You have read the in-kernel documentation about how we handle CVEs, right? It's listed there :)
thanks,
greg k-h
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 28ead3eaabc16ecc907cfb71876da028080f6356
WARNING: Author mismatch between patch and upstream commit: Backport author: hsimeliere.opensource@witekio.com Commit author: Xu Kuohaixukuohai@huawei.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 5d5e3b4cbe8e) 6.1.y | Not found
Note: The patch differs from the upstream commit: --- 1: 28ead3eaabc16 ! 1: d2cd989f36650 bpf: Prevent tail call between progs attached to different hooks @@ Metadata ## Commit message ## bpf: Prevent tail call between progs attached to different hooks
+ [ 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 @@ Commit message Link: https://lore.kernel.org/r/20240719110059.797546-4-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Andrii Nakryiko andrii@kernel.org + [ 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@se.com + Signed-off-by: Hugo SIMELIERE hsimeliere.opensource@witekio.com
## include/linux/bpf.h ## @@ include/linux/bpf.h: struct bpf_map { @@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map,
if (fp->kprobe_override) return false; -@@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map, - * in the case of devmap and cpumap). Until device checks - * are implemented, prohibit adding dev-bound programs to program maps. - */ -- if (bpf_prog_is_dev_bound(fp->aux)) -+ if (bpf_prog_is_dev_bound(aux)) - return false; - - spin_lock(&map->owner.lock); @@ kernel/bpf/core.c: bool bpf_prog_map_compatible(struct bpf_map *map, */ map->owner.type = prog_type; ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Greg KH -
hsimeliere.opensource＠witekio.com -
Sasha Levin