Hi Mimi, On 7/27/21 10:32 PM, Mimi Zohar wrote: > [Cc'ing Paul Moore] > > Hi Simon, > [snip] > >> + >> + 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 > >> + 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
