Skip to content

Commit

Permalink
zfs_ioctl: saved_poolname can be truncated
Browse files Browse the repository at this point in the history
As it uses kmem_strdup() and kmem_strfree() which both rely on
strlen() being the same, but saved_poolname can be truncated causing:

SPL: kernel memory allocator:
buffer freed to wrong cache
SPL: buffer was allocated from kmem_alloc_16,
SPL: caller attempting free to kmem_alloc_8.
SPL: buffer=0xffffff90acc66a38  bufctl=0x0  cache: kmem_alloc_8

Signed-off-by: Jorgen Lundman <lundman@lundman.net>
  • Loading branch information
lundman committed Jun 16, 2020
1 parent 883a40f commit dcb0998
Showing 1 changed file with 17 additions and 5 deletions.
22 changes: 17 additions & 5 deletions module/zfs/zfs_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -7380,6 +7380,7 @@ zfsdev_ioctl_common(uint_t vecnum, zfs_cmd_t *zc, int flag)
int error, cmd;
const zfs_ioc_vec_t *vec;
char *saved_poolname = NULL;
size_t saved_poolname_len = 0;
nvlist_t *innvl = NULL;
fstrans_cookie_t cookie;

Expand Down Expand Up @@ -7479,11 +7480,22 @@ zfsdev_ioctl_common(uint_t vecnum, zfs_cmd_t *zc, int flag)
goto out;

/* legacy ioctls can modify zc_name */
saved_poolname = kmem_strdup(zc->zc_name);
/*
* Can't use kmem_strup() as we might truncate the string and
* kmem_strfree() would then free with incorrect size.
* Further complicated by that it is used with tsd_set() and
* freed over in zfs_ioc_log_history(kmem_strfree()) so the
* size of saved_poolname has to be the exact truncated version
*/
saved_poolname_len = strlen(zc->zc_name) + 1;
saved_poolname = kmem_alloc(saved_poolname_len, KM_SLEEP);

/* All platforms use KM_SLEEP so NULL shouldn't happen? */
if (saved_poolname == NULL) {
error = SET_ERROR(ENOMEM);
goto out;
} else {
strlcpy(saved_poolname, zc->zc_name, saved_poolname_len);
saved_poolname[strcspn(saved_poolname, "/@#")] = '\0';
}

Expand Down Expand Up @@ -7562,11 +7574,11 @@ zfsdev_ioctl_common(uint_t vecnum, zfs_cmd_t *zc, int flag)
char *s = tsd_get(zfs_allow_log_key);
if (s != NULL)
kmem_strfree(s);
(void) tsd_set(zfs_allow_log_key, saved_poolname);
} else {
if (saved_poolname != NULL)
kmem_strfree(saved_poolname);
(void) tsd_set(zfs_allow_log_key, kmem_strdup(saved_poolname));
}
if (saved_poolname != NULL)
kmem_free(saved_poolname, saved_poolname_len);

return (error);
}

Expand Down

0 comments on commit dcb0998

Please sign in to comment.