Re: [PATCH] TaskTracker : Simplified thread information tracker.

Thank you for comments. Richard Guy Briggs wrote: > Steve already mentioned any user-influenced fields need to be escaped, > so I'd recommend audit_log_untrustedstring() as being much simpler from > your perspective and much better tested and understood from audit > maintainer's perspective. At least use the existing 'o' printf format > specifier instead of inventing your own. I do acknowledge that the > resulting output from your function is easier to read in its raw format > passed from the kernel, however, it makes your code harder to maintain. I'm not sure whether I should use audit_log_untrustedstring(). This record contains multiple user-influenced comm names. If I use audit_log_untrustedstring(), I would need to split this record into multiple records like history[0]='...' history[1]='...' history[2]='...' in order to avoid matching delimiters (i.e. ';', '=' and '>') used in this record. This would also change from "char *" in "struct task_struct" to array of struct { "comm name", "pid", "stamp" } in "struct task_struct". I don't know whether such change makes easier to maintain than now.

> As for the date-stamping bits, they seem to be the majority of the code > in audit_update_history(). I'd just emit a number and punt that to > userspace for decoding. Alternatively, I'd use an existing service in > the kernel to do that date formatting, or at least call a new function > to format that date string should a suitable one not already exist, so > you can remove that complexity from audit_update_history(). Since I don't know existing functions for date formatting, I split it as a new function. If it is acceptable, I'd like to make that function public and replace tomoyo_convert_time() in security/tomoyo/util.c with that function.

diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 072566d..2edeba2 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1344,6 +1354,17 @@ out: audit_log_end(ab); } +static void audit_log_history(struct audit_context *context) +{ + struct audit_buffer *ab; + + ab = audit_log_start(context, GFP_KERNEL, AUDIT_PROCHISTORY); + if (!ab) + return; /* audit_panic or being filtered */ + audit_log_format(ab, "history='%s'", current->comm_history);

+ audit_log_end(ab); +} + static void audit_log_exit(struct audit_context *context, struct task_struct *tsk) {

+void audit_update_history(void) + *cp++ = '\\'; + *cp++ = (c >> 6) + '0'; + *cp++ = ((c >> 3) & 7) + '0'; + *cp++ = (c & 7) + '0';

+ /* Append timestamp. */ + { + struct yyyymmdd_hhmmss stamp; + + tt_get_time(&stamp); + cp += snprintf(cp, buf - cp + sizeof(buf) - 1, + ";start=%04u%02u%02u%02u%02u%02u", stamp.year, + stamp.month, stamp.day, stamp.hour, stamp.min, + stamp.sec); + }