hal/hisi: fix strcpy-param-overlap in get_hisi_sdk (UB, caught by ASAN)#173
Merged
Conversation
get_hisi_sdk() reformats the /proc/umap/sys version line into
"<version> (<build time>)". line_from_file() returns the *greedy* capture
of `Version: \[(.+)\]`, so on a typical Hisilicon line such as
[SYS] Version: [Hi3516CV500_MPP_V2.0.2.1 B030 Release], Build Time[May 28 2020, 11:04:35]
buf ends up spanning BOTH brackets. The code overwrites the first ']' with
" (" and then splices the build time in via strcpy(ptr, build + 1) -- but
build+1 and ptr alias the same buffer, so that is an overlapping copy.
strcpy() with overlapping ranges is undefined behaviour; it only happens to
produce the right string on glibc because the copy direction reads before it
writes. AddressSanitizer aborts on it (strcpy-param-overlap), and a different
libc/arch could silently corrupt the string.
Use memmove(), which is well-defined for overlap and yields the identical
"<version> (<build time>)" result.
Found by running an ASAN-instrumented build on a live Hi3516CV500 (glibc)
camera; after the fix the full run completes ASAN-clean with
sdk: "Hi3516CV500_MPP_V2.0.2.1 B030 Release (May 28 2020, 11:04:35)".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
What
get_hisi_sdk()reformats the/proc/umap/sysversion line into"<version> (<build time>)". Becauseline_from_file()returns the greedy capture ofVersion: \[(.+)\], a typical Hisilicon line like:leaves
bufspanning both brackets. The code overwrites the first]with" ("and then splices the build time in with:build + 1andptrpoint into the samebuf, so this is an overlappingstrcpy— undefined behaviour. It only happens to produce the right string on glibc because the copy direction reads-before-writes; AddressSanitizer aborts on it (strcpy-param-overlap), and a different libc/arch could silently corrupt the string.Fix
Use
memmove(), which is well-defined for overlapping ranges and yields the identical output.How it was found / verified
Running an ASAN-instrumented build on a live Hi3516CV500 (glibc) camera aborted here:
After the fix, the full run completes ASAN-clean (exit 0) and the value is unchanged:
🤖 Generated with Claude Code