Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Fix TOCTOU race in zpool_do_labelclear() 

Coverity reported a TOCTOU race in `zpool_do_labelclear()`. This is not
believed to be a real security issue, but fixing it reduces the number
of syscalls we do and will prevent other static analyzers from
complaining about this.

The code is expected to be equivalent. However, under rare
circumstances, such as ELOOP, ENAMETOOLONG, ENOMEM, ENOTDIR and
EOVERFLOW, we will display the error message that we currently display
for the `open()` syscall rather than the one that we currently display
for the `stat()` syscall. This is considered to be an improvement.

Reported-by: Coverity (CID-1524188)
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
This commit is contained in:
Richard Yao 2022-12-04 17:42:43 -05:00 committed by Brian Behlendorf
parent a8d83e2a24
commit 9368b3877c
1 changed files with 20 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
cmd/zpool/zpool_main.c
View File

@ -1288,7 +1288,6 @@ zpool_do_labelclear(int argc, char **argv)
{ {
char vdev[MAXPATHLEN]; char vdev[MAXPATHLEN];
char *name = NULL; char *name = NULL;
struct stat st;
int c, fd = -1, ret = 0; int c, fd = -1, ret = 0;
nvlist_t *config; nvlist_t *config;
pool_state_t state; pool_state_t state;
@ -1321,14 +1320,20 @@ zpool_do_labelclear(int argc, char **argv)
usage(B_FALSE); usage(B_FALSE);
} }
(void) strlcpy(vdev, argv[0], sizeof (vdev));
/* /*
* Check if we were given absolute path and use it as is. * If we cannot open an absolute path, we quit.
* Otherwise if the provided vdev name doesn't point to a file, * Otherwise if the provided vdev name doesn't point to a file,
* try prepending expected disk paths and partition numbers. * try prepending expected disk paths and partition numbers.
*/ */
(void) strlcpy(vdev, argv[0], sizeof (vdev)); if ((fd = open(vdev, O_RDWR)) < 0) {
if (vdev[0] != '/' && stat(vdev, &st) != 0) {
int error; int error;
if (vdev[0] == '/') {
(void) fprintf(stderr, gettext("failed to open "
"%s: %s\n"), vdev, strerror(errno));
return (1);
}
error = zfs_resolve_shortname(argv[0], vdev, MAXPATHLEN); error = zfs_resolve_shortname(argv[0], vdev, MAXPATHLEN);
if (error == 0 && zfs_dev_is_whole_disk(vdev)) { if (error == 0 && zfs_dev_is_whole_disk(vdev)) {
@ -1336,20 +1341,21 @@ zpool_do_labelclear(int argc, char **argv)
error = ENOENT; error = ENOENT;
} }
if (error || (stat(vdev, &st) != 0)) { if (error || ((fd = open(vdev, O_RDWR)) < 0)) {
(void) fprintf(stderr, gettext( if (errno == ENOENT) {
"failed to find device %s, try specifying absolute " (void) fprintf(stderr, gettext(
"path instead\n"), argv[0]); "failed to find device %s, try "
"specifying absolute path instead\n"),
argv[0]);
return (1);
}
(void) fprintf(stderr, gettext("failed to open %s:"
" %s\n"), vdev, strerror(errno));
return (1); return (1);
} }
} }
if ((fd = open(vdev, O_RDWR)) < 0) {
(void) fprintf(stderr, gettext("failed to open %s: %s\n"),
vdev, strerror(errno));
return (1);
}
/* /*
* Flush all dirty pages for the block device. This should not be * Flush all dirty pages for the block device. This should not be
* fatal when the device does not support BLKFLSBUF as would be the * fatal when the device does not support BLKFLSBUF as would be the