Messages in this thread | | | Subject | Re: [PATCH V2] xfs: fix string handling in get/set functions | From | Eric Sandeen <> | Date | Wed, 6 Jun 2018 12:45:33 -0500 |
| |
On 6/6/18 5:58 AM, Christoph Hellwig wrote: >> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c >> index 82f7c83c1dad..596e176c19a6 100644 >> --- a/fs/xfs/xfs_ioctl.c >> +++ b/fs/xfs/xfs_ioctl.c >> @@ -1828,13 +1828,13 @@ xfs_ioc_getlabel( >> /* Paranoia */ >> BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX); >> >> + /* 1 larger than sb_fname, so this ensures a trailing NUL char */ >> + memset(label, 0, sizeof(label)); > > I don't get the comment. In fact I don't even get why we need any > comment here. This is a structure that gets copied to userspace, > and we zero the whole structure, as we should do by default for > anything that goes to userspace.
Sure, I guess we didn't really need the comment, my main point was that we were guaranteed to have a \0 remaining at the end because we'd never copy over it due to sb_fname's smaller size.
> Otherwise this looks fine to me.
Thanks. In retrospect we could have gone back to Arnd's original patch but I guess the explicit memset is ... well, nice and explicit.
-Eric
| |