FIX: Wrong sizeof argument of coverity defects(CID 147639) #5198
Conversation
| @@ -548,7 +548,7 @@ is_shorthand_path(const char *arg, char *path, | |||
| return (0); | |||
| } | |||
|
|
|||
| strlcpy(path, arg, sizeof (path)); | |||
| strlcpy(path, arg, path_size); | |||
There was a problem hiding this comment.
Yeah, this is clearly wrong and we need to use a passed size. Similarly you should pass the new path_size to zfs_resolve_shortname() above and remove the assumption that the buffer is MAXPATHLEN.
|
@GeLiXin can you incorporate the review feedback and rebase this on master. |
GeLiXin
force-pushed
the
wrongsize
branch
2 times, most recently
from
459a29c to
9a64b9e
Compare
|
@behlendorf I'm really sorry for feedback for such a long time, as I just come back after a 7 day's holiday of Chinese national day. |
| @@ -660,7 +660,8 @@ make_leaf_vdev(nvlist_t *props, const char *arg, uint64_t is_log) | |||
| /* After is_whole_disk() check restore original passed path */ | |||
| strlcpy(path, arg, MAXPATHLEN); | |||
| } else { | |||
| err = is_shorthand_path(arg, path, &statbuf, &wholedisk); | |||
| err = is_shorthand_path(arg, path, MAXPATHLEN, | |||
| &statbuf, &wholedisk); | |||
There was a problem hiding this comment.
[style] Four space indent for continued lines.
+ err = is_shorthand_path(arg, path, MAXPATHLEN,
+ &statbuf, &wholedisk);
44a4e2e to
56a1807
Compare
|
@behlendorf I modified a little to fit different path size which not assumpt it to be MAXPATHLEN any more. |
|
@GeLiXin I should have been clearer. Continued lines should be indented by four spaces after being tab aligned. I posted the fixed line, you'll need to convert that last tab to four space instead. You can see this same style used in the rest of the file whenever a line is continued. The removal of MAXPATHLEN in favor of sizeof is fine. |
| } else { | ||
| err = is_shorthand_path(arg, path, &statbuf, &wholedisk); | ||
| err = is_shorthand_path(arg, path, sizeof (path), | ||
| &statbuf, &wholedisk); |
There are 4 defects of wrong size/sizeof argument. CID 147639?147635?147634 seems like false positives. But 147639 seems really risky: When array is passed as parameter, it degenerate into pointer, so the sizeof(path) in is_shorthand_path() always get return value of 8, not the string lenght we want. Signed-off-by: GeLiXin <[email protected]>
|
@behlendorf I get your point now, thanks. And I'm done! |
There are 4 defects of wrong size/sizeof argument.
CID 147634, 147635, 147636 seems like false positives.
But 147639 seems really risky:
When array is passed as parameter, it degenerate into pointer,
so the
sizeof(path)inis_shorthand_path()always get return value of 8, not the string lenght we want.Signed-off-by: GeLiXin [email protected]