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

Fix undefined behavior in spa_sync_props() 

8eae2d214c caused Coverity to begin
complaining about "Improper use of negative value" in two places in
spa_sync_props() because Coverity correctly inferred from `prop ==
ZPOOL_PROP_INVAL` that prop could be -1 while both zpool_prop_to_name()
and zpool_prop_get_type() use it an array index, which is undefined
behavior.

Assuming that the system does not panic from an attempt to read invalid
memory, the case statement for ZPOOL_PROP_INVAL will ensure that only
user properties will reach this code when prop is ZPOOL_PROP_INVAL, such
that execution will continue safely. However, if we are unlucky enough
to read invalid memory, then the system will panic.

This issue predates the patch that caused coverity to begin complaining.
Thankfully, our userland tools do not pass nonsense to us, so this bug
should not be triggered unless a future userland tool attempts to set a
property that we do not understand.

Reported-by: Coverity (CID-1561129)
Reported-by: Coverity (CID-1561130)
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: George Amanakis <gamanakis@gmail.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14860
This commit is contained in:
Richard Yao 2023-05-12 17:10:14 -04:00 committed by Brian Behlendorf
parent c87798d8ff
commit ee7b71dbc9
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
module/zfs/spa.c
View File

@ -8942,12 +8942,12 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
}
/* normalize the property name */
propname = zpool_prop_to_name(prop);
proptype = zpool_prop_get_type(prop);
if (prop == ZPOOL_PROP_INVAL &&
zfs_prop_user(elemname)) {
if (prop == ZPOOL_PROP_INVAL) {
propname = elemname;
proptype = PROP_TYPE_STRING;
} else {
propname = zpool_prop_to_name(prop);
proptype = zpool_prop_get_type(prop);
}
if (nvpair_type(elem) == DATA_TYPE_STRING) {