dummymbuf: Avoid copyout of uninitialized memory from the sysctl handler

[markj]

Summary

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
Fixes: 8aaffd78c0f5 ("Add dummymbuf module for testing purposes")

[markj]

sys/net/dummymbuf.c

95

BTW, shouldn't we try to parse the rule here, so the user can get an error if something is wrong?

[igoro]

Looks great to me.

[igoro]

sys/net/dummymbuf.c

95

Really, it looks like an interesting idea to improve UX, and I'm thinking about possible trade-offs with extra code and related complexity added. From current use cases it's expected to read-and-parse the rules soon after they are set. Additional UX questions come as well like whether we should write it anyway or rollback to the previous good state, that probably needs an extra play with buffers. I need to revise the sysctl side, probably the only feedback it can yield is an exit code, while the rules parser complains to the console.

Which approach do you believe would be most effective in this situation?

[markj]

sys/net/dummymbuf.c

95

To me, the most natural thing to do is copy the new string in to a spare buffer, try to parse it, and return EINVAL if the parse fails. In this case, the existing rules should be left alone. I think this is "typical" behaviour for such a sysctl handler. One example which comes to mind is sysctl_rules(), in mac_portacl(4).

[igoro]

sys/net/dummymbuf.c

95

Sure, let's do it. I see that sysctl(8) reports it nicely and I like the idea even more:
sysctl: net.dummymbuf.rules=abc: Invalid argument

It looks I've never set wrong values via sysctl or do not remember a case :)

Please, find it here: https://reviews.freebsd.org/D46496. It's expected to go after your patch, I will update it if required.