libmultipath: check get_word() result in disassemble_map() - #155
libmultipath: check get_word() result in disassemble_map()#155wallycheng wants to merge 1 commit into
Conversation
|
@mwilck Hi, Could you take a look please? Thanks! |
bmarzins
left a comment
bmarzins
left a comment
There was a problem hiding this comment.
Thanks for the patch! It looks good other than two nitpicks.
- Would you mind changing the commit message? This is not just a problem on OOM (which is unlikely to happen in practice). A malformed multipath table, where the last path has no arguments, will also cause a crash, and that seems like a more likely thing to happen.
- since we know word is NULL, we might as well just jump to
out, instead ofout1.
0559c86 to
237cf0f
Compare
Thanks for the review! Updated the commit message to mention malformed tables as well. and switched to "goto out" as well. |
|
@bmarzins Hi — The PR’s checks are stuck as pending; I don’t have permission to re-run workflows. Could someone with repo access please re-run the CI or advise? Thanks! |
|
@wallycheng, don't worry about the PR checks. I will apply your patch in my "tip" branch. From there it will be merged to the "queue" branch and ultimately into the next release. |
|
Hm. Your patch contains an extra semicolon causing the CI failures. I have fixed this now, but it shows that you didn't even compile-test your change. Please do that next time. |
get_word() returns 0 and sets *word to NULL not only when calloc()
fails, but also when the input table is malformed(e.g, the last path
has no arguments). In both cases, dereferencing word later in
atoi(word) causes a NULL pointer dereference.
Add a check for !word and jump to out since word is already NULL,
matching the error-handling pattern used elsewhere in the function.
Signed-off-by: wallycheng <295412260@qq.com>
237cf0f to
48fa855
Compare
|
Thanks！I've fixed it in the latest commit. |
3 participants
get_word() leaves *word == NULL and returns 0 if its internal
calloc() fails, which can happen under memory pressure. Every
other call site in disassemble_map() checks for this case, but the
path-arguments loop skipped the check before calling atoi(word),
causing a NULL-pointer dereference on the OOM path.
Jump to the existing out1 label, which already frees word, to
match the rest of the function and bail out cleanly.