linux-kernel - Re: [PATCH -next] fs-verity: Use struct_size() helper in fsverity_ioctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 19 May 2022 09:50:38 +0800
From:   "zhangjianhua (E)" <chris.zjh@...wei.com>
To:     Eric Biggers <ebiggers@...nel.org>
CC:     <tytso@....edu>, <linux-fscrypt@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next] fs-verity: Use struct_size() helper in
 fsverity_ioctl_measure()

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 <chris.zjh@...wei.com>
>> ---
>>   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
> .