On Fri, 2019-12-20 at 08:48 -0800, Lakshmi Ramasubramanian wrote:

On 12/20/2019 8:31 AM, Florent Revest wrote:

quoted

  
+/**
+ * ima_file_hash - return the stored measurement if a file has
been hashed.
+ * @file: pointer to the file
+ * @buf: buffer in which to store the hash
+ * @buf_size: length of the buffer
+ *
+ * On success, output the hash into buf and return the hash
algorithm (as
+ * defined in the enum hash_algo).
+ * If the hash is larger than buf, then only size bytes will be
copied. It
+ * generally just makes sense to pass a buffer capable of holding
the largest
+ * possible hash: IMA_MAX_DIGEST_SIZE

If the given buffer is smaller than the hash length, wouldn't it be 
better to return the required size and a status indicating the buffer
is not enough. The caller can then call back with the required
buffer.

If the hash is truncated the caller may not know if the hash is
partial or not.

I agree with Mimi's answer that the caller would know based on the
returned hash algorithm.

quoted

+ *
+ * If IMA is disabled or if no measurement is available, return
-EOPNOTSUPP.
+ * If the parameters are incorrect, return -EINVAL.
+ */
+int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+{
+	struct inode *inode;
+	struct integrity_iint_cache *iint;
+	size_t copied_size;
+
+	if (!file || !buf)
+		return -EINVAL;
+
+	if (!ima_policy_flag)
+		return -EOPNOTSUPP;
+
+	inode = file_inode(file);
+	iint = integrity_iint_find(inode);
+	if (!iint)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&iint->mutex);
+	copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
+	memcpy(buf, iint->ima_hash->digest, copied_size);
+	mutex_unlock(&iint->mutex);
+
+	return iint->ima_hash->algo;

Should the hash algorithm be copied from iinit->ima_hash to a local 
variable while holding the mutex and that one returned?

I assume iinit->mutex  is taken to ensure iinit->ima_hash is not
removed while this function is accessing it.

Ah! Good catch, thank you :)