Messages in this thread | | | Subject | Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings | From | Miloslav Trmač <> | Date | Thu, 11 Sep 2008 16:43:19 +0200 |
| |
Thanks for the review. Eric Paris píše v Čt 11. 09. 2008 v 10:25 -0400: > On Thu, 2008-09-11 at 00:23 +0200, Miloslav Trmač wrote: > > This patch modifies audit_log_n_untrustedstring() to only log the data > > before the first NUL byte, if any. > > I'm going to have to say NAK on this patch. > > It's still not right looking at the other user, > audit_log_single_execve_arg(). An execve arg with a NULL could loose > the stuff after the NULL (not break the record like audit_tty) since the > execve uses %s rather than calling trusted string. execve() arguments are NUL-terminated strings: audit_log_single_execve_arg() starts with len_left = len = strnlen_user(p, MAX_ARG_STRLEN) - 1; and the two cycles handle chunks in the first "len" bytes of "p". audit_log_single_execve_arg() never touches any bytes after the first NUL.
So, when audit_log_single_execve_arg() calls audit_string_contains_control(buf, to_send), strlen(buf) == to_send and the patch does not change anything.
> How about we change the meaning of audit_string_contains_control() > return values? If it returns positive that is the number of bytes in a > legitimate string up to the first null. -1 means it is hex. That is possible, although the return value convention is somewhat complex. Anyway, no change to the semantics of audit_string_contains_control() is necessary for execve() argument logging. Mirek
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |