| Message ID | 20220518093829.2248801-1-chris.zjh@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next] fs-verity: Use struct_size() helper in fsverity_ioctl_measure() | expand |
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@huawei.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
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@huawei.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 > .
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))
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@huawei.com> --- fs/verity/measure.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)