Make use of the struct_size() helper instead of an open-coded version,
in order to avoid any potential type mistakes or integer overflows that,
in the worst scenario, could lead to heap overflows.
Also, address the following sparse warnings:
fs/verity/measure.c:48:9: warning: using sizeof on a flexible structure
fs/verity/measure.c:52:38: warning: using sizeof on a flexible structure
Signed-off-by: Zhang Jianhua <[email protected]>
---
fs/verity/measure.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index e99c00350c28..4a388116d0de 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -27,6 +27,7 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
const struct fsverity_info *vi;
const struct fsverity_hash_alg *hash_alg;
struct fsverity_digest arg;
+ size_t arg_size = struct_size(&arg, digest, 0);
vi = fsverity_get_info(inode);
if (!vi)
@@ -44,11 +45,11 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
if (arg.digest_size < hash_alg->digest_size)
return -EOVERFLOW;
- memset(&arg, 0, sizeof(arg));
+ memset(&arg, 0, arg_size);
arg.digest_algorithm = hash_alg - fsverity_hash_algs;
arg.digest_size = hash_alg->digest_size;
- if (copy_to_user(uarg, &arg, sizeof(arg)))
+ if (copy_to_user(uarg, &arg, arg_size))
return -EFAULT;
if (copy_to_user(uarg->digest, vi->file_digest, hash_alg->digest_size))
--
2.31.0
On Wed, May 18, 2022 at 05:38:29PM +0800, Zhang Jianhua wrote:
> Make use of the struct_size() helper instead of an open-coded version,
> in order to avoid any potential type mistakes or integer overflows that,
> in the worst scenario, could lead to heap overflows.
>
> Also, address the following sparse warnings:
> fs/verity/measure.c:48:9: warning: using sizeof on a flexible structure
> fs/verity/measure.c:52:38: warning: using sizeof on a flexible structure
>
> Signed-off-by: Zhang Jianhua <[email protected]>
> ---
> fs/verity/measure.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> index e99c00350c28..4a388116d0de 100644
> --- a/fs/verity/measure.c
> +++ b/fs/verity/measure.c
> @@ -27,6 +27,7 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
> const struct fsverity_info *vi;
> const struct fsverity_hash_alg *hash_alg;
> struct fsverity_digest arg;
> + size_t arg_size = struct_size(&arg, digest, 0);
>
> vi = fsverity_get_info(inode);
> if (!vi)
> @@ -44,11 +45,11 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
> if (arg.digest_size < hash_alg->digest_size)
> return -EOVERFLOW;
>
> - memset(&arg, 0, sizeof(arg));
> + memset(&arg, 0, arg_size);
> arg.digest_algorithm = hash_alg - fsverity_hash_algs;
> arg.digest_size = hash_alg->digest_size;
>
> - if (copy_to_user(uarg, &arg, sizeof(arg)))
> + if (copy_to_user(uarg, &arg, arg_size))
> return -EFAULT;
'arg' is just a stack variable that doesn't use the flexible array field. So
this change on its own is pretty pointless and just obfuscates the code.
If it's nevertheless worth it to get rid of the sparse warning, to make the
wider codebase clean of this class of warning, we could still do it anyway. But
please make the commit message correctly say that the purpose is just to
eliminate the sparse warning, and don't incorrectly claim that the code "could
lead to heap overflows".
- Eric
Thanks, I will modify the commit message and send the next version.
Zhang Jianhua
在 2022/5/19 1:48, Eric Biggers 写道:
> On Wed, May 18, 2022 at 05:38:29PM +0800, Zhang Jianhua wrote:
>> Make use of the struct_size() helper instead of an open-coded version,
>> in order to avoid any potential type mistakes or integer overflows that,
>> in the worst scenario, could lead to heap overflows.
>>
>> Also, address the following sparse warnings:
>> fs/verity/measure.c:48:9: warning: using sizeof on a flexible structure
>> fs/verity/measure.c:52:38: warning: using sizeof on a flexible structure
>>
>> Signed-off-by: Zhang Jianhua <[email protected]>
>> ---
>> fs/verity/measure.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
>> index e99c00350c28..4a388116d0de 100644
>> --- a/fs/verity/measure.c
>> +++ b/fs/verity/measure.c
>> @@ -27,6 +27,7 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
>> const struct fsverity_info *vi;
>> const struct fsverity_hash_alg *hash_alg;
>> struct fsverity_digest arg;
>> + size_t arg_size = struct_size(&arg, digest, 0);
>>
>> vi = fsverity_get_info(inode);
>> if (!vi)
>> @@ -44,11 +45,11 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
>> if (arg.digest_size < hash_alg->digest_size)
>> return -EOVERFLOW;
>>
>> - memset(&arg, 0, sizeof(arg));
>> + memset(&arg, 0, arg_size);
>> arg.digest_algorithm = hash_alg - fsverity_hash_algs;
>> arg.digest_size = hash_alg->digest_size;
>>
>> - if (copy_to_user(uarg, &arg, sizeof(arg)))
>> + if (copy_to_user(uarg, &arg, arg_size))
>> return -EFAULT;
> 'arg' is just a stack variable that doesn't use the flexible array field. So
> this change on its own is pretty pointless and just obfuscates the code.
>
> If it's nevertheless worth it to get rid of the sparse warning, to make the
> wider codebase clean of this class of warning, we could still do it anyway. But
> please make the commit message correctly say that the purpose is just to
> eliminate the sparse warning, and don't incorrectly claim that the code "could
> lead to heap overflows".
>
> - Eric
> .