-
Notifications
You must be signed in to change notification settings - Fork 9
[ciqlts8_6] Patches for CVE-2022-23222 #335
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
base: ciqlts8_6
Are you sure you want to change the base?
Conversation
First push with a build, pushed so I have a history before a rebase (if any). Will fill in the fields once I have the logs. |
Your first five commits are part of what was originally a nine commit series:
Your other three commits were part of a backport to 5.15
If you look at the commit history for fips-8-compliant/4.18.0-553.16.1 you'll see RH came up with something pretty close to the commit list above. They didn't backport the selftest from the first series or
I think I'd be tempted to do it just like above. Interestingly the commit history there marks Just my .02 |
Yeah, I spent yesterday trying to find my way out of at least one patch and came to the same conclusion as you. Only thing remaining now is to properly backport patches. They were done out of order and I don't trust |
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit d639b9d upstream-diff A merge confict arised because the function `bpf_free_kfunc_btf_tab` introduced in 2357672 ("bpf: Introduce BPF support for kernel module function calls") does not exist in our tree. There are some common properties shared between bpf reg, ret and arg values. For instance, a value may be a NULL pointer, or a pointer to a read-only memory. Previously, to express these properties, enumeration was used. For example, in order to test whether a reg value can be NULL, reg_type_may_be_null() simply enumerates all types that are possibly NULL. The problem of this approach is that it's not scalable and causes a lot of duplication. These properties can be combined, for example, a type could be either MAYBE_NULL or RDONLY, or both. This patch series rewrites the layout of reg_type, arg_type and ret_type, so that common properties can be extracted and represented as composable flag. For example, one can write ARG_PTR_TO_MEM | PTR_MAYBE_NULL which is equivalent to the previous ARG_PTR_TO_MEM_OR_NULL The type ARG_PTR_TO_MEM are called "base type" in this patch. Base types can be extended with flags. A flag occupies the higher bits while base types sits in the lower bits. This patch in particular sets up a set of macro for this purpose. The following patches will rewrite arg_types, ret_types and reg_types respectively. Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit d639b9d) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit 48946bd We have introduced a new type to make bpf_arg composable, by reserving high bits of bpf_arg to represent flags of a type. One of the flags is PTR_MAYBE_NULL which indicates a pointer may be NULL. When applying this flag to an arg_type, it means the arg can take NULL pointer. This patch switches the qualified arg_types to use this flag. The arg_types changed in this patch include: 1. ARG_PTR_TO_MAP_VALUE_OR_NULL 2. ARG_PTR_TO_MEM_OR_NULL 3. ARG_PTR_TO_CTX_OR_NULL 4. ARG_PTR_TO_SOCKET_OR_NULL 5. ARG_PTR_TO_ALLOC_MEM_OR_NULL 6. ARG_PTR_TO_STACK_OR_NULL This patch does not eliminate the use of these arg_types, instead it makes them an alias to the 'ARG_XXX | PTR_MAYBE_NULL'. Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 48946bd) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit 3c48073 upstream-diff A merge confict arised because 3e8ce29 ("bpf: Prevent pointer mismatch in bpf_timer_init.") does not exist in our tree. We have introduced a new type to make bpf_ret composable, by reserving high bits to represent flags. One of the flag is PTR_MAYBE_NULL, which indicates a pointer may be NULL. When applying this flag to ret_types, it means the returned value could be a NULL pointer. This patch switches the qualified arg_types to use this flag. The ret_types changed in this patch include: 1. RET_PTR_TO_MAP_VALUE_OR_NULL 2. RET_PTR_TO_SOCKET_OR_NULL 3. RET_PTR_TO_TCP_SOCK_OR_NULL 4. RET_PTR_TO_SOCK_COMMON_OR_NULL 5. RET_PTR_TO_ALLOC_MEM_OR_NULL 6. RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL 7. RET_PTR_TO_BTF_ID_OR_NULL This patch doesn't eliminate the use of these names, instead it makes them aliases to 'RET_PTR_TO_XXX | PTR_MAYBE_NULL'. Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 3c48073) Signed-off-by: Pratham Patel <[email protected]>
e0bb719
to
d6c9f52
Compare
Updated the PR comment with relevant details. Only thing to note is that the local branch is called |
kernel/bpf/verifier.c
Outdated
@@ -9095,10 +9052,11 @@ static int check_return_code(struct bpf_verifier_env *env) | |||
} | |||
|
|||
reg = cur_regs(env) + BPF_REG_0; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This obviously doesn't hurt anything, but I'm not sure how we got this extra line here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the result of a git-cherry-pick
that wasn't part of a merge conflict. Sorry about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good!
Before going through commit by commit I looked at the bpf changes in this commit from fips-8-compliant/4.18.0-553.16.1 because it ecapsulates all of the changes you've made here. The only differences I saw were the additional whitespace (noted in the comment above) and the lack of changes to check_helper_mem_access because you didn't pick up the commit below:
eb88e636998a4 bpf: Check PTR_TO_MEM | MEM_RDONLY in check_helper_mem_access
I think I probably would have picked that one up too just so that the code here is in sync with fips-8, but I don't think its specifically pertinent to the CVE in question so I think its ok to leave it off.
The individual commits look good to me too. Get rid of that extra line and I think I'm ready to rubber stamp this. Thanks!
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit c25b2ae upstream-diff A merge confict arised because bfc6bb7 ("bpf: Implement verifier support for validation of async callbacks.") and 3e8ce29 ("bpf: Prevent pointer mismatch in bpf_timer_init.") do not exist in our tree. We have introduced a new type to make bpf_reg composable, by allocating bits in the type to represent flags. One of the flags is PTR_MAYBE_NULL which indicates a pointer may be NULL. This patch switches the qualified reg_types to use this flag. The reg_types changed in this patch include: 1. PTR_TO_MAP_VALUE_OR_NULL 2. PTR_TO_SOCKET_OR_NULL 3. PTR_TO_SOCK_COMMON_OR_NULL 4. PTR_TO_TCP_SOCK_OR_NULL 5. PTR_TO_BTF_ID_OR_NULL 6. PTR_TO_MEM_OR_NULL 7. PTR_TO_RDONLY_BUF_OR_NULL 8. PTR_TO_RDWR_BUF_OR_NULL Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/r/[email protected] (cherry picked from commit c25b2ae) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit 20b2aff This patch introduce a flag MEM_RDONLY to tag a reg value pointing to read-only memory. It makes the following changes: 1. PTR_TO_RDWR_BUF -> PTR_TO_BUF 2. PTR_TO_RDONLY_BUF -> PTR_TO_BUF | MEM_RDONLY Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 20b2aff) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit cf9f2f8 Remove PTR_TO_MEM_OR_NULL and replace it with PTR_TO_MEM combined with flag PTR_MAYBE_NULL. Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit cf9f2f8) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit 34d3a78 Tag the return type of {per, this}_cpu_ptr with RDONLY_MEM. The returned value of this pair of helpers is kernel object, which can not be updated by bpf programs. Previously these two helpers return PTR_OT_MEM for kernel objects of scalar type, which allows one to directly modify the memory. Now with RDONLY_MEM tagging, the verifier will reject programs that write into RDONLY_MEM. Fixes: 63d9b80 ("bpf: Introducte bpf_this_cpu_ptr()") Fixes: eaa6bcb ("bpf: Introduce bpf_per_cpu_ptr()") Fixes: 4976b71 ("bpf: Introduce pseudo_btf_id") Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 34d3a78) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Hao Luo <[email protected]> commit 216e3cd upstream-diff A merge confict arised because several commits were introduced since linux-4.18.y untill this commit (216e3cd ("bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.")) was merged upstream. Not listing all commits because there are 20+ such commits. Some helper functions may modify its arguments, for example, bpf_d_path, bpf_get_stack etc. Previously, their argument types were marked as ARG_PTR_TO_MEM, which is compatible with read-only mem types, such as PTR_TO_RDONLY_BUF. Therefore it's legitimate, but technically incorrect, to modify a read-only memory by passing it into one of such helper functions. This patch tags the bpf_args compatible with immutable memory with MEM_RDONLY flag. The arguments that don't have this flag will be only compatible with mutable memory types, preventing the helper from modifying a read-only memory. The bpf_args that have MEM_RDONLY are compatible with both mutable memory and immutable memory. Signed-off-by: Hao Luo <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 216e3cd) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 jira VULN-33337 cve CVE-2022-48929 commit-author Kumar Kartikeya Dwivedi <[email protected]> commit 45ce4b4 upstream-diff A merge confict arised because 3363bd0 ("bpf: Extend kfunc with PTR_TO_CTX, PTR_TO_MEM argument support") does not exist in our tree. When commit e6ac245 ("bpf: Support bpf program calling kernel function") added kfunc support, it defined reg2btf_ids as a cheap way to translate the verifier reg type to the appropriate btf_vmlinux BTF ID, however commit c25b2ae ("bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL") moved the __BPF_REG_TYPE_MAX from the last member of bpf_reg_type enum to after the base register types, and defined other variants using type flag composition. However, now, the direct usage of reg->type to index into reg2btf_ids may no longer fall into __BPF_REG_TYPE_MAX range, and hence lead to out of bounds access and kernel crash on dereference of bad pointer. Fixes: c25b2ae ("bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL") Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Link: https://lore.kernel.org/bpf/[email protected] (cherry picked from commit 45ce4b4) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Daniel Borkmann <[email protected]> commit be80a1d upstream-diff A merge confict arised because 3363bd0 ("bpf: Extend kfunc with PTR_TO_CTX, PTR_TO_MEM argument support") does not exist in our tree Generalize the check_ctx_reg() helper function into a more generic named one so that it can be reused for other register types as well to check whether their offset is non-zero. No functional change. Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: John Fastabend <[email protected]> Acked-by: Alexei Starovoitov <[email protected]> (cherry picked from commit be80a1d) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 pre-cve CVE-2022-23222 commit-author Daniel Borkmann <[email protected]> commit 6788ab2 Right now the assertion on check_ptr_off_reg() is only enforced for register types PTR_TO_CTX (and open coded also for PTR_TO_BTF_ID), however, this is insufficient since many other PTR_TO_* register types such as PTR_TO_FUNC do not handle/expect register offsets when passed to helper functions. Given this can slip-through easily when adding new types, make this an explicit allow-list and reject all other current and future types by default if this is encountered. Also, extend check_ptr_off_reg() to handle PTR_TO_BTF_ID as well instead of duplicating it. For PTR_TO_BTF_ID, reg->off is used for BTF to match expected BTF ids if struct offset is used. This part still needs to be allowed, but the dynamic off from the tnum must be rejected. Fixes: 69c087b ("bpf: Add bpf_for_each_map_elem() helper") Fixes: eaa6bcb ("bpf: Introduce bpf_per_cpu_ptr()") Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: John Fastabend <[email protected]> Acked-by: Alexei Starovoitov <[email protected]> (cherry picked from commit 6788ab2) Signed-off-by: Pratham Patel <[email protected]>
jira VULN-140 cve CVE-2022-23222 commit-author Daniel Borkmann <[email protected]> commit 64620e0 Both bpf_ringbuf_submit() and bpf_ringbuf_discard() have ARG_PTR_TO_ALLOC_MEM in their bpf_func_proto definition as their first argument. They both expect the result from a prior bpf_ringbuf_reserve() call which has a return type of RET_PTR_TO_ALLOC_MEM_OR_NULL. Meaning, after a NULL check in the code, the verifier will promote the register type in the non-NULL branch to a PTR_TO_MEM and in the NULL branch to a known zero scalar. Generally, pointer arithmetic on PTR_TO_MEM is allowed, so the latter could have an offset. The ARG_PTR_TO_ALLOC_MEM expects a PTR_TO_MEM register type. However, the non- zero result from bpf_ringbuf_reserve() must be fed into either bpf_ringbuf_submit() or bpf_ringbuf_discard() but with the original offset given it will then read out the struct bpf_ringbuf_hdr mapping. The verifier missed to enforce a zero offset, so that out of bounds access can be triggered which could be used to escalate privileges if unprivileged BPF was enabled (disabled by default in kernel). Fixes: 457f443 ("bpf: Implement BPF ring buffer and verifier support for it") Reported-by: <[email protected]> (SecCoder Security Lab) Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: John Fastabend <[email protected]> Acked-by: Alexei Starovoitov <[email protected]> (cherry picked from commit 64620e0) Signed-off-by: Pratham Patel <[email protected]>
d6c9f52
to
a9dee97
Compare
Rebased and pushed removing the unnecessary newline. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bpf: Fix crash due to out of bounds access into reg2btf_ids.
jira VULN-140
pre-cve https://github.com/advisories/GHSA-jjwq-78g3-m34w
jira VULN-33337
cve https://github.com/advisories/GHSA-5752-wxxv-m9xhbpf: Fix crash due to out of bounds access into reg2btf_ids.
jira VULN-140
pre-cve https://github.com/advisories/GHSA-jjwq-78g3-m34w
jira VULN-33337
cve https://github.com/advisories/GHSA-5752-wxxv-m9xh
While the idea is make sense to address CVE-2022-2322
since its an independent CVE we can just label it as CVE-2022-48929
and the corresponding jira only.
These are actually this CVE: CVE-2022-0500 which is a CVE with 7 commits all of which are included below.
feel free to change the header of these to:
Drop the
|
Kernel build logs
build.log
Kselftests
kselftest-before.log
kselftest-after.log