mkimg: Fix parsing of filenames containing colons#2041
mkimg: Fix parsing of filenames containing colons#2041kitkat1424 wants to merge 2 commits intofreebsd:mainfrom
Conversation
kitkat1424
force-pushed
the
fix/mkimg-colon-parsing
branch
from
7448a97 to
8800453
Compare
When using PART_KIND_FILE (-p type:=filename), mkimg uses a colon to separate an optional offset (e.g., file:offset). strsep() was being used to split the string at the first colon. This caused failures when the filename itself contained a colon (e.g., "th:is"). This patch use stat() to check if the entire string exists as a file. If so, use it directly without splitting. If the full string is not a valid file, fall back to splitting at the right-most colon using strrchr(). PR: \<257960\> Signed-off-by: Aaditya Singh <aadityavksingh@gmail.com>
kitkat1424
force-pushed
the
fix/mkimg-colon-parsing
branch
from
8800453 to
1fe1f5c
Compare
jlduran
left a comment
jlduran
left a comment
There was a problem hiding this comment.
Thank you. I think this is a good approach. I believe it is only missing the path when it is a directory, and should error.
Also, per style(9), the braces around single-line statements are optional, and in the case of the current style of the file, should not be used.
Consider this a pre-review, while we wait for the CODEOWNER.
usr.bin/mkimg/mkimg.c
Outdated
| case PART_KIND_FILE: | ||
| size = part->contents; | ||
| if (stat(part->contents, &sb) == 0 && | ||
| !S_ISDIR(sb.st_mode)) { |
There was a problem hiding this comment.
What happens when it is a directory?
There was a problem hiding this comment.
For an existing directory, the program would have fallen through to the next switch statement. The image_copyin function returns an EISDIR error (specifically, when it eventually attempts a read(fd, ...) on the directory's file descriptor).
In my 2nd commit, I have added a change so that it returns with this error right away to avoid some unnecessary operations.
Incorrect directories, just like incorrect filenames were already handled (they will return an error on open()).
|
Thank you for taking the time to contribute to FreeBSD! All issues resolved. |
kitkat1424
force-pushed
the
fix/mkimg-colon-parsing
branch
from
9113936 to
f7cd337
Compare
|
Thanks for taking the time to look at this @jlduran I have made some changes to address your feedback regarding |
Use errc() to fail and exit immediately when an existing directory is input instead of a file in PART_KIND_FILE mode. Signed-off-by: Aaditya Singh <aadityavksingh@gmail.com>
kitkat1424
force-pushed
the
fix/mkimg-colon-parsing
branch
from
f7cd337 to
74de4d4
Compare
2 participants
When using PART_KIND_FILE (-p type:=filename), mkimg uses a colon to separate an optional offset (e.g., file:offset). strsep() was being used to split the string at the first colon. This caused failures when the filename itself contained a colon (e.g., "th:is").
This patch use stat() to check if the entire string exists as a file. If so, use it directly without splitting.
If the full string is not a valid file, fall back to splitting at the right-most colon using strrchr().
PR: 257960
Signed-off by: Aaditya Singh aadityavksingh@gmail.com