libibverbs: Avoid memcpy from NULL in fill_attr_in() #1669
+3
−2
Conversation
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
zengyijing
force-pushed
the
fix-fill-attr-zero-len
branch
from
807eb70 to
65a5630
Compare
Member
|
Actually, using NULL as a pointer is declared as undefined behavior in C99 spec, section 7.1.4. What are the compilers and static analyze tools which complain? |
Contributor
Author
|
@rleon UndefinedBehaviorSanitizer (UBSan) complains about invalid-null-argument during run time |
Member
And this more makes sense and align with C99 specification. You should update your commit and description to check for NULL pointer and not for zero length. |
zengyijing
force-pushed
the
fix-fill-attr-zero-len
branch
from
65a5630 to
99a724f
Compare
zengyijing
changed the title
Contributor
Author
|
@rleon the code & commit msg are updated |
rleon
reviewed
Jan 21, 2026
fill_attr_in() unconditionally calls memcpy() when len <= sizeof(u64), regardless of whether the data pointer is NULL. In commit d9af497 ("verbs: Add ibv_cmd_alloc/free commands for DMA handle"), the call fill_attr_in_enum(cmdb, UVERBS_ATTR_ALLOC_DMAH_TPH_MEM_TYPE, attr->tph_mem_type, NULL, 0); started passing a NULL data pointer together with len == 0, which leads to memcpy() being invoked with a NULL source address. While nothing is actually copied, some compilers and sanitizers treat this as undefined behavior and emit errors. Fix this by only inlining small attributes when len <= sizeof(u64) and the data pointer is non-NULL. In all other cases, including zero-length attributes with a NULL data pointer, the pointer is stored via ioctl_ptr_to_u64() instead. This preserves the existing behavior for valid callers while avoiding memcpy() from NULL. fill_attr_in() was originally introduced in commit c344635 ("verbs: Add basic infrastructure support for the kabi ioctl"). Fixes: d9af497 ("verbs: Add ibv_cmd_alloc/free commands for DMA handle") Signed-off-by: Yijing Zeng <[email protected]>
zengyijing
force-pushed
the
fix-fill-attr-zero-len
branch
from
99a724f to
1de4f92
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
fill_attr_in() unconditionally calls memcpy() when len <= sizeof(u64), even if len is zero. In commit d9af497 ("verbs: Add ibv_cmd_alloc/free commands for DMA handle"), the call
started passing a NULL data pointer together with len == 0, which leads to memcpy() being invoked with a NULL source address. While nothing is actually copied, some compilers and sanitizers treat this as undefined behavior and emit errors.
Avoid this by skipping memcpy() when len is zero. Zero-length attributes have no payload, so this does not change behavior.
fill_attr_in() was originally introduced in commit c344635 ("verbs: Add basic infrastructure support for the kabi ioctl").
Fixes: d9af497 ("verbs: Add ibv_cmd_alloc/free commands for DMA handle")
Signed-off-by: Yijing Zeng [email protected]