Date: Sun, 1 Sep 2024 14:10:02 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 61295e098599 - main - dummymbuf: Avoid copyout of uninitialized memory from the sysctl handler Message-ID: <202409011410.481EA2H5002061@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=61295e09859953cce5140daf9c2ff85b3feb0e74 commit 61295e09859953cce5140daf9c2ff85b3feb0e74 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2024-08-31 01:19:09 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2024-09-01 14:09:53 +0000 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} <ifname> <opname>[ <opargs>];" - " ...;"); + "{inet | inet6 | ethernet} {in | out} <ifname> <opname>[<opargs>]; ...;"); /* * Statistics
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202409011410.481EA2H5002061>