bpf: Set kfunc dynptr arg type flag based on prototype#11640
Closed
ameryhung wants to merge 1 commit intokernel-patches:bpf-next_basefrom
Closed
bpf: Set kfunc dynptr arg type flag based on prototype#11640ameryhung wants to merge 1 commit intokernel-patches:bpf-next_basefrom
ameryhung wants to merge 1 commit intokernel-patches:bpf-next_basefrom
Conversation
The verifer currently tracks the mutability of a kfunc's bpf_dynptr through its signature, where "const struct bpf_dynptr *" means the structure itself is immutable. However, many existing kfunc prototypes are inconsistent with the definition. For example, bpf_dynptr_adjust() mutates a dynptr's start and offset but marks the argument as a const pointer, while many other kfunc that does not mutate the dynptr but mark themselves as mutable. In addition, the verifier currently does not honor the const qualifier in the prototype as it currently determines whether tagging the arg_type with MEM_RDONLY based on the register type. Since all the verifier care is to prevent CONST_PTR_TO_DYNPTR from being destroyed in callback and global subprogram, redefine the mutability at the bpf_dynptr level to just bpf_dynptr_kern->data. Then, explicitly prohibit passing CONST_PTR_TO_DYNPTR to an argument tagged with MEM_UNINIT or OBJ_RELEASE. The mutability of a dynptr's view is not really interesting so drop MEM_RDONLY annotation from the helpers and kfuncs. In process_dynptr_func(), when passing CONST_PTR_TO_DYNPTR to dynptr argument that is not __uninit, check against OBJ_RELEASE instead of MEM_RDONLY and drop a now identical check in umark_stack_slots_dynptr(). Note that the prototype of various kfuncs are still fixed to maintain the semantic correctness in the C source code. Adding or removing the const qualifier does not break backward compatibility. In test_kfunc_dynptr_param.c, initialize dynptr to 0 to avoid -Wuninitialized-const-pointer warning. Signed-off-by: Amery Hung <ameryhung@gmail.com>
ameryhung
force-pushed
the
dynptr_proto_v1
branch
from
6a8ad50 to
c721d2d
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
21 times, most recently
from
ef1cb9f to
3842f6d
Compare
|
Automatically cleaning up stale PR; feel free to reopen if needed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The verifer currently tracks the mutability of a kfunc's bpf_dynptr through its signature, where "const struct bpf_dynptr *" means the structure itself is immutable. However, many existing kfunc prototypes are inconsistent with the definition. For example, bpf_dynptr_adjust() mutates a dynptr's start and offset but marks the argument as a const pointer, while many other kfunc that does not mutate the dynptr but mark themselves as mutable. In addition, the verifier currently does not honor the const qualifier in the prototype as it currently determines whether tagging the arg_type with MEM_RDONLY based on the register type.
Since all the verifier care is to prevent CONST_PTR_TO_DYNPTR from being destroyed in callback and global subprogram, redefine the mutability at the bpf_dynptr level to just bpf_dynptr_kern->data. Then, explicitly prohibit passing CONST_PTR_TO_DYNPTR to an argument tagged with MEM_UNINIT or OBJ_RELEASE. The mutability of a dynptr's view is not really interesting so drop MEM_RDONLY annotation from the helpers and kfuncs.
In process_dynptr_func(), when passing CONST_PTR_TO_DYNPTR to dynptr argument that is not __uninit, check against OBJ_RELEASE instead of MEM_RDONLY and drop a now identical check in umark_stack_slots_dynptr().
Note that the prototype of various kfuncs are still fixed to maintain the semantic correctness in the C source code. Adding or removing the const qualifier does not break backward compatibility.