summaryrefslogtreecommitdiff
path: root/fs
diff options
context:
space:
mode:
authorOndrej Mosnacek <omosnace@redhat.com>2023-02-10 15:58:23 +0100
committerLuis Chamberlain <mcgrof@kernel.org>2023-02-21 13:34:07 -0800
commitf1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 (patch)
tree59a95f2b5b687281e48ae76f73293b72d082f139 /fs
parent1b72607d7321e66829e11148712b3a2ba1dc83e7 (diff)
sysctl: fix proc_dobool() usability
Currently proc_dobool expects a (bool *) in table->data, but sizeof(int) in table->maxsize, because it uses do_proc_dointvec() directly. This is unsafe for at least two reasons: 1. A sysctl table definition may use { .data = &variable, .maxsize = sizeof(variable) }, not realizing that this makes the sysctl unusable (see the Fixes: tag) and that they need to use the completely counterintuitive sizeof(int) instead. 2. proc_dobool() will currently try to parse an array of values if given .maxsize >= 2*sizeof(int), but will try to write values of type bool by offsets of sizeof(int), so it will not work correctly with neither an (int *) nor a (bool *). There is no .maxsize validation to prevent this. Fix this by: 1. Constraining proc_dobool() to allow only one value and .maxsize == sizeof(bool). 2. Wrapping the original struct ctl_table in a temporary one with .data pointing to a local int variable and .maxsize set to sizeof(int) and passing this one to proc_dointvec(), converting the value to/from bool as needed (using proc_dou8vec_minmax() as an example). 3. Extending sysctl_check_table() to enforce proc_dobool() expectations. 4. Fixing the proc_dobool() docstring (it was just copy-pasted from proc_douintvec, apparently...). 5. Converting all existing proc_dobool() users to set .maxsize to sizeof(bool) instead of sizeof(int). Fixes: 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled") Fixes: a2071573d634 ("sysctl: introduce new proc handler proc_dobool") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> Acked-by: Kees Cook <keescook@chromium.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/lockd/svc.c2
-rw-r--r--fs/proc/proc_sysctl.c6
2 files changed, 7 insertions, 1 deletions
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 59ef8a1f843f..914ea1c3537d 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -496,7 +496,7 @@ static struct ctl_table nlm_sysctls[] = {
{
.procname = "nsm_use_hostnames",
.data = &nsm_use_hostnames,
- .maxlen = sizeof(int),
+ .maxlen = sizeof(bool),
.mode = 0644,
.proc_handler = proc_dobool,
},
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index e89bd8f1368b..5851eb5bc726 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1124,6 +1124,11 @@ static int sysctl_check_table_array(const char *path, struct ctl_table *table)
err |= sysctl_err(path, table, "array not allowed");
}
+ if (table->proc_handler == proc_dobool) {
+ if (table->maxlen != sizeof(bool))
+ err |= sysctl_err(path, table, "array not allowed");
+ }
+
return err;
}
@@ -1136,6 +1141,7 @@ static int sysctl_check_table(const char *path, struct ctl_table *table)
err |= sysctl_err(path, entry, "Not a file");
if ((entry->proc_handler == proc_dostring) ||
+ (entry->proc_handler == proc_dobool) ||
(entry->proc_handler == proc_dointvec) ||
(entry->proc_handler == proc_douintvec) ||
(entry->proc_handler == proc_douintvec_minmax) ||