On Thu, Dec 31, 2015 at 03:24:53PM +1100, Stephen Rothwell wrote:
> Hi James,
>
> Today's linux-next merge of the security tree got a conflict in:
>
> security/integrity/ima/ima_fs.c
>
> between commit:
>
> 3bc8f29b149e ("new helper: memdup_user_nul()")
>
> from the vfs tree and commit:
>
> 38d859f991f3 ("IMA: policy can now be updated multiple times")
>
> from the security tree.
>
> I fixed it up (hopefully, see below) and can carry the fix as necessary
> (no action is required).
> + res = mutex_lock_interruptible(&ima_write_mutex);
> + if (res)
> + return res;
>
> if (datalen >= PAGE_SIZE)
> datalen = PAGE_SIZE - 1;
>
> /* No partial writes. */
> + result = -EINVAL;
> if (*ppos != 0)
> - return -EINVAL;
> + goto out;
>
> - result = -ENOMEM;
> - data = kmalloc(datalen + 1, GFP_KERNEL);
> - if (!data)
> - goto out;
> -
> - *(data + datalen) = '\0';
> -
> - result = -EFAULT;
> - if (copy_from_user(data, buf, datalen))
> + data = memdup_user_nul(buf, datalen);
> - if (IS_ERR(data))
> - return PTR_ERR(data);
> ++ if (IS_ERR(data)) {
> ++ result = PTR_ERR(data);
> + goto out;
> ++ }
Why do it in this order? With or without opencoding memdup_user_nul(),
what's the point of taking the mutex before copying the data from
userland? All it achieves is holding it longer, over the area that
needs no exclusion whatsoever.
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
