On Wed, Jul 20, 2016 at 2:43 PM, Paul Moore <[email protected]> wrote: > From: Paul Moore <[email protected]> > > There is a double fetch problem in audit_log_single_execve_arg() > where we first check the execve(2) argumnets for any "bad" characters > which would require hex encoding and then re-fetch the arguments for > logging in the audit record[1]. Of course this leaves a window of > opportunity for an unsavory application to munge with the data. > > This patch reworks things by only fetching the argument data once[2] > into a buffer where it is scanned and logged into the audit > records(s). In addition to fixing the double fetch, this patch > improves on the original code in a few other ways: better handling > of large arguments which require encoding, stricter record length > checking, and some performance improvements (completely unverified, > but we got rid of some strlen() calls, that's got to be a good > thing). > > As part of the development of this patch, I've also created a basic > regression test for the audit-testsuite, the test can be tracked on > GitHub at the following link: > > * https://github.com/linux-audit/audit-testsuite/issues/25 > > [1] If you pay careful attention, there is actually a triple fetch > problem due to a strnlen_user() call at the top of the function. > > [2] This is a tiny white lie, we do make a call to strnlen_user() > prior to fetching the argument data. I don't like it, but due to the > way the audit record is structured we really have no choice unless we > copy the entire argument at once (which would require a rather > wasteful allocation). The good news is that with this patch the > kernel no longer relies on this strnlen_user() value for anything > beyond recording it in the log, we also update it with a trustworthy > value whenever possible. > > Reported-by: Pengfei Wang <[email protected]> > Cc: <[email protected]> > Signed-off-by: Paul Moore <[email protected]> > --- > kernel/auditsc.c | 332 > +++++++++++++++++++++++++++--------------------------- > 1 file changed, 164 insertions(+), 168 deletions(-)
Generally I would take a patch this late for the upcoming merge window, but considering the nature of this patch I'm going to make an exception. I've added it to the audit#next branch and I've built a temporary kernel for testing at the link below for anyone who may be interested; I'm also building a new pcmoore/kernel-secnext COPR kernel with the patch right now, it should be available in a few hours. * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/394059 -- paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
