From 61295e09859953cce5140daf9c2ff85b3feb0e74 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Sat, 31 Aug 2024 01:19:09 +0000 Subject: [PATCH] dummymbuf: Avoid copyout of uninitialized memory from the sysctl handler If *rulesp was initially unset, we'll allocate a new buffer and pass it to sysctl_handle_string(), which copies the existing string out and then copies in the new string. We need to make sure the buffer containing the existing rules is initialized, otherwise we leak kernel memory to userspace. Fix some nearby style nits while here. Reported by: KMSAN Reviewed by: igoro, kp Fixes: 8aaffd78c0f5 ("Add dummymbuf module for testing purposes") Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D46493 --- sys/net/dummymbuf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sys/net/dummymbuf.c b/sys/net/dummymbuf.c index 8c46421888ed..d4ba00b13235 100644 --- a/sys/net/dummymbuf.c +++ b/sys/net/dummymbuf.c @@ -74,7 +74,7 @@ dmb_sysctl_handle_rules(SYSCTL_HANDLER_ARGS) char **rulesp = (char **)arg1; if (req->newptr == NULL) { - // read only + /* read only */ DMB_RULES_SLOCK(); arg1 = *rulesp; if (arg1 == NULL) { @@ -84,10 +84,12 @@ dmb_sysctl_handle_rules(SYSCTL_HANDLER_ARGS) error = sysctl_handle_string(oidp, arg1, arg2, req); DMB_RULES_SUNLOCK(); } else { - // read and write + /* read and write */ DMB_RULES_XLOCK(); - if (*rulesp == NULL) - *rulesp = malloc(arg2, M_DUMMYMBUF_RULES, M_WAITOK); + if (*rulesp == NULL) { + *rulesp = malloc(arg2, M_DUMMYMBUF_RULES, + M_WAITOK | M_ZERO); + } arg1 = *rulesp; error = sysctl_handle_string(oidp, arg1, arg2, req); DMB_RULES_XUNLOCK(); @@ -99,8 +101,7 @@ dmb_sysctl_handle_rules(SYSCTL_HANDLER_ARGS) SYSCTL_PROC(_net_dummymbuf, OID_AUTO, rules, CTLTYPE_STRING | CTLFLAG_MPSAFE | CTLFLAG_RW | CTLFLAG_VNET, &VNET_NAME(dmb_rules), RULES_MAXLEN, dmb_sysctl_handle_rules, "A", - "{inet | inet6 | ethernet} {in | out} [ ];" - " ...;"); + "{inet | inet6 | ethernet} {in | out} []; ...;"); /* * Statistics