Re: [PATCH 2/8] integrity: IMA as an integrity service provider
I'm very slow to the game, I know, but today was the first kernel that I
built from linux-next with IMA on. I have a comment, and hopefully more
to come....
On Fri, 2009-02-06 at 14:52 -0500, Mimi Zohar wrote:
+void integrity_audit_msg(int audit_msgno, struct inode *inode,
+ const unsigned char *fname, const char *op,
+ const char *cause, int result, int audit_info)
+{
+ struct audit_buffer *ab;
+
+ if (!ima_audit && audit_info == 1) /* Skip informational messages */
+ return;
+
+ ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno);
+ audit_log_format(ab, "integrity: pid=%d uid=%u auid=%u",
+ current->pid, current->cred->uid,
+ audit_get_loginuid(current));
+ audit_log_task_context(ab);
+ switch (audit_msgno) {
+ case AUDIT_INTEGRITY_DATA:
+ case AUDIT_INTEGRITY_METADATA:
+ case AUDIT_INTEGRITY_PCR:
+ audit_log_format(ab, " op=%s cause=%s", op, cause);
There are cases where (at least) cause can be a multiword string. These
should be using audit_log_string() instead of %s. I'm starting to frown
on %s in audit log more and more heavily these days, actually I don't
think I should let any more users of %s in at all. If you want I'll go
through and change all of these, but I want to make sure that you will
be ok with multiword string being either "" or translated. An example
of the problem I see would be in say ima_update_policy() where you have
const char *cause = "already exists"; Currently that record will get
emitted like:
blah cause=already exists blah
which screws up parsers. It should be:
blah cause="already exists" blah
make sense?
+ break;
+ case AUDIT_INTEGRITY_HASH:
+ audit_log_format(ab, " op=%s hash=%s", op, cause);
+ break;
+ case AUDIT_INTEGRITY_STATUS:
+ default:
+ audit_log_format(ab, " op=%s", op);
+ }
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, current->comm);
+ if (fname) {
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fname);
+ }
+ if (inode)
+ audit_log_format(ab, " dev=%s ino=%lu",
+ inode->i_sb->s_id, inode->i_ino);
+ audit_log_format(ab, " res=%d", result);
+ audit_log_end(ab);