Skip to content

Commit

Permalink
zfs: Fix SPA sysctl handlers
Browse files Browse the repository at this point in the history
sbuf_cpy() resets the sbuf state, which is wrong for sbufs allocated by
sbuf_new_for_sysctl().  In particular, this code triggers an assertion
failure in sbuf_clear().

Simplify by just using sysctl_handle_string() for both reading and
setting the tunable.

Apply to FreeBSD directly since this bug causes "sysctl -a" to crash the
kernel.

PR:		276039
Reported by:	pho
Reviewed by:	mav
Pull Request:	openzfs/zfs#15719
  • Loading branch information
markjdb authored and bsdjhb committed Mar 13, 2024
2 parents a07a6b9 + 09af4bf commit 71b528c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 48 deletions.
30 changes: 6 additions & 24 deletions sys/contrib/openzfs/module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1445,8 +1445,6 @@ spa_taskq_write_param_get(char *buf, zfs_kernel_param_t *kp)
return (spa_taskq_param_get(ZIO_TYPE_WRITE, buf));
}
#else
#include <sys/sbuf.h>

/*
* On FreeBSD load-time parameters can be set up before malloc() is available,
* so we have to do all the parsing work on the stack.
Expand All @@ -1457,19 +1455,11 @@ static int
spa_taskq_read_param(ZFS_MODULE_PARAM_ARGS)
{
char buf[SPA_TASKQ_PARAM_MAX];
int err = 0;

if (req->newptr == NULL) {
int len = spa_taskq_param_get(ZIO_TYPE_READ, buf);
struct sbuf *s = sbuf_new_for_sysctl(NULL, NULL, len+1, req);
sbuf_cpy(s, buf);
err = sbuf_finish(s);
sbuf_delete(s);
return (err);
}
int err;

(void) spa_taskq_param_get(ZIO_TYPE_READ, buf);
err = sysctl_handle_string(oidp, buf, sizeof (buf), req);
if (err)
if (err || req->newptr == NULL)
return (err);
return (spa_taskq_param_set(ZIO_TYPE_READ, buf));
}
Expand All @@ -1478,19 +1468,11 @@ static int
spa_taskq_write_param(ZFS_MODULE_PARAM_ARGS)
{
char buf[SPA_TASKQ_PARAM_MAX];
int err = 0;

if (req->newptr == NULL) {
int len = spa_taskq_param_get(ZIO_TYPE_WRITE, buf);
struct sbuf *s = sbuf_new_for_sysctl(NULL, NULL, len+1, req);
sbuf_cpy(s, buf);
err = sbuf_finish(s);
sbuf_delete(s);
return (err);
}
int err;

(void) spa_taskq_param_get(ZIO_TYPE_WRITE, buf);
err = sysctl_handle_string(oidp, buf, sizeof (buf), req);
if (err)
if (err || req->newptr == NULL)
return (err);
return (spa_taskq_param_set(ZIO_TYPE_WRITE, buf));
}
Expand Down
30 changes: 6 additions & 24 deletions sys/contrib/subrepo-openzfs/module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1445,8 +1445,6 @@ spa_taskq_write_param_get(char *buf, zfs_kernel_param_t *kp)
return (spa_taskq_param_get(ZIO_TYPE_WRITE, buf));
}
#else
#include <sys/sbuf.h>

/*
* On FreeBSD load-time parameters can be set up before malloc() is available,
* so we have to do all the parsing work on the stack.
Expand All @@ -1457,19 +1455,11 @@ static int
spa_taskq_read_param(ZFS_MODULE_PARAM_ARGS)
{
char buf[SPA_TASKQ_PARAM_MAX];
int err = 0;

if (req->newptr == NULL) {
int len = spa_taskq_param_get(ZIO_TYPE_READ, buf);
struct sbuf *s = sbuf_new_for_sysctl(NULL, NULL, len+1, req);
sbuf_cpy(s, buf);
err = sbuf_finish(s);
sbuf_delete(s);
return (err);
}
int err;

(void) spa_taskq_param_get(ZIO_TYPE_READ, buf);
err = sysctl_handle_string(oidp, buf, sizeof (buf), req);
if (err)
if (err || req->newptr == NULL)
return (err);
return (spa_taskq_param_set(ZIO_TYPE_READ, buf));
}
Expand All @@ -1478,19 +1468,11 @@ static int
spa_taskq_write_param(ZFS_MODULE_PARAM_ARGS)
{
char buf[SPA_TASKQ_PARAM_MAX];
int err = 0;

if (req->newptr == NULL) {
int len = spa_taskq_param_get(ZIO_TYPE_WRITE, buf);
struct sbuf *s = sbuf_new_for_sysctl(NULL, NULL, len+1, req);
sbuf_cpy(s, buf);
err = sbuf_finish(s);
sbuf_delete(s);
return (err);
}
int err;

(void) spa_taskq_param_get(ZIO_TYPE_WRITE, buf);
err = sysctl_handle_string(oidp, buf, sizeof (buf), req);
if (err)
if (err || req->newptr == NULL)
return (err);
return (spa_taskq_param_set(ZIO_TYPE_WRITE, buf));
}
Expand Down

0 comments on commit 71b528c

Please sign in to comment.