Hi Simon, On Wed, 2021-07-28 at 07:00 +0000, THOBY Simon wrote: > >> + > >> + 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 ? Memory pressure isn't the reason for preventing the xattr write. It's the reason for not being able to audit the setxattr failure. thanks, Mimi
