Hi Mimi,

On 7/27/21 10:32 PM, Mimi Zohar wrote:

[Cc'ing Paul Moore]

Hi Simon,

[snip]

quoted

+
+	if (likely(dentry_hash == ima_hash_algo
+	    || crypto_has_alg(hash_algo_name[dentry_hash], 0, 0)))
+		return 0;
+
+	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
+	/* no memory available ? no file path for you */

The comment here is unnecessary.  Avoid or limit comments inside a
function.  Refer to the section "8) Commenting" in
Documentation/process/coding-style.rst

quoted

+	if (pathbuf)
+		path = dentry_path(dentry, pathbuf, PATH_MAX);
+
+	/* disallow xattr writes with algorithms not built in the kernel */
+	integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry),
+		path, "collect_data", "unavailable-hash-algorithm", res, 0);

This will emit an audit message without the filename when !path.  Is
this what you intended?

This is what I was clumsily trying to explain in the previous comment: if we cannot
allocate memory for a file path, I thought it best to log the audit message without
the path than fail with a -ENOMEM (auditing will also try to allocate a memory buffer
too, but a bit smaller, and memory could have been reclaimed between the two calls,
so the auditing operation may succeed).

Of course I could also return -ENOMEM, and it would happily propagate back to the user.

What do you think ?

Thanks,
Simon