Hi, I'm investigating a race in audit_log_untrusted_string() in the case of task_struct::comm.
Originally from commit 0a4ff8c2 audit_log_task() currently hands task_struct::comm directly to audit_log_untrusted_string() which can race if another task/thread on another CPU modifies it between the time strlen() is called on it and the time it is processed in audit_log_n_untrustedstring(). I originally thought it was just a matter of the value being truncated or mixed between old and new values, but it appears it is worse than that in that it causes following labels to be dropped. If it just affected the value of comm that was printed, I am guessing this wouldn't be a big deal since the user can modify this value anyway and we never did trust it. however, since it affects the rest of the line, it has to be addressed. In commit 219f0817 from 2005, Stephen Smalley used get_task_comm() to get the value of comm to log to audit_log_untrusted_string() which calls the task_lock. This is quite safe, but incurs overhead that may not be found acceptable by some. Eric subsequently used memcpy() in c2a7780e in 2008 in another area of the code that stores the value for later rather than use it immediately, so the race issue was not present there, but there is still the danger of incomplete or mixed text in that field. Both are safe in terms of avoiding losing other fields. One might have half-inconsistent information in it, the other incurs locking costs. I'm inclined to go get_task_comm() in all 5 locations, but if we care more about locking overhead, I'll switch to memcpy(). Steve, do we care about the integrity of the comm field? Eric, is the overhead of task_lock unacceptable? Linked in this thread (should I be able to convince git send-email to work with me) please find the get_task_comm() patch and the alternate memcpy() patch. - RGB -- Richard Guy Briggs <rbri...@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
