* [PATCH 4/6] xfs: use memdup_user() [not found] <49DC4CC0.9050805@cn.fujitsu.com> @ 2009-04-08 7:08 ` Li Zefan 2009-04-08 13:22 ` Christoph Hellwig 0 siblings, 1 reply; 5+ messages in thread From: Li Zefan @ 2009-04-08 7:08 UTC (permalink / raw) To: felixb; +Cc: Andrew Morton, LKML, xfs Remove open-coded memdup_user() Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> --- fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++---------------- fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++-------- 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c index d0b4994..34eaab6 100644 --- a/fs/xfs/linux-2.6/xfs_ioctl.c +++ b/fs/xfs/linux-2.6/xfs_ioctl.c @@ -489,17 +489,12 @@ xfs_attrmulti_attr_set( if (len > XATTR_SIZE_MAX) return EINVAL; - kbuf = kmalloc(len, GFP_KERNEL); - if (!kbuf) - return ENOMEM; - - if (copy_from_user(kbuf, ubuf, len)) - goto out_kfree; + kbuf = memdup_user(ubuf, len); + if (IS_ERR(kbuf)) + return PTR_ERR(kbuf); error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags); - out_kfree: - kfree(kbuf); return error; } @@ -540,20 +535,16 @@ xfs_attrmulti_by_handle( if (!size || size > 16 * PAGE_SIZE) goto out_dput; - error = ENOMEM; - ops = kmalloc(size, GFP_KERNEL); - if (!ops) + ops = memdup_user(am_hreq.ops, size); + if (IS_ERR(ops)) { + error = PTR_ERR(ops); goto out_dput; - - error = EFAULT; - if (copy_from_user(ops, am_hreq.ops, size)) - goto out_kfree_ops; + } attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL); if (!attr_name) goto out_kfree_ops; - error = 0; for (i = 0; i < am_hreq.opcount; i++) { ops[i].am_error = strncpy_from_user(attr_name, diff --git a/fs/xfs/linux-2.6/xfs_ioctl32.c b/fs/xfs/linux-2.6/xfs_ioctl32.c index c70c4e3..0882d16 100644 --- a/fs/xfs/linux-2.6/xfs_ioctl32.c +++ b/fs/xfs/linux-2.6/xfs_ioctl32.c @@ -427,20 +427,16 @@ xfs_compat_attrmulti_by_handle( if (!size || size > 16 * PAGE_SIZE) goto out_dput; - error = ENOMEM; - ops = kmalloc(size, GFP_KERNEL); - if (!ops) + ops = memdup_user(compat_ptr(am_hreq.ops), size); + if (IS_ERR(ops)) { + error = PTR_ERR(ops); goto out_dput; - - error = EFAULT; - if (copy_from_user(ops, compat_ptr(am_hreq.ops), size)) - goto out_kfree_ops; + } attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL); if (!attr_name) goto out_kfree_ops; - error = 0; for (i = 0; i < am_hreq.opcount; i++) { ops[i].am_error = strncpy_from_user(attr_name, -- 1.5.4.rc3 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 4/6] xfs: use memdup_user() 2009-04-08 7:08 ` [PATCH 4/6] xfs: use memdup_user() Li Zefan @ 2009-04-08 13:22 ` Christoph Hellwig 2009-04-08 17:31 ` Roland Dreier 2009-04-08 20:06 ` Felix Blyakher 0 siblings, 2 replies; 5+ messages in thread From: Christoph Hellwig @ 2009-04-08 13:22 UTC (permalink / raw) To: Li Zefan; +Cc: Andrew Morton, LKML, xfs On Wed, Apr 08, 2009 at 03:08:04PM +0800, Li Zefan wrote: > Remove open-coded memdup_user() > > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> > --- > fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++---------------- > fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++-------- > 2 files changed, 11 insertions(+), 24 deletions(-) > > diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c > index d0b4994..34eaab6 100644 > --- a/fs/xfs/linux-2.6/xfs_ioctl.c > +++ b/fs/xfs/linux-2.6/xfs_ioctl.c > @@ -489,17 +489,12 @@ xfs_attrmulti_attr_set( > if (len > XATTR_SIZE_MAX) > return EINVAL; > > - kbuf = kmalloc(len, GFP_KERNEL); > - if (!kbuf) > - return ENOMEM; > - > - if (copy_from_user(kbuf, ubuf, len)) > - goto out_kfree; > + kbuf = memdup_user(ubuf, len); > + if (IS_ERR(kbuf)) > + return PTR_ERR(kbuf); wouldn't NULL be a better error return for this kind of interface, matching kmalloc? Otherwise the patch looks good to me. Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/6] xfs: use memdup_user() 2009-04-08 13:22 ` Christoph Hellwig @ 2009-04-08 17:31 ` Roland Dreier 2009-04-09 0:43 ` Li Zefan 2009-04-08 20:06 ` Felix Blyakher 1 sibling, 1 reply; 5+ messages in thread From: Roland Dreier @ 2009-04-08 17:31 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Andrew Morton, Li Zefan, LKML, xfs > wouldn't NULL be a better error return for this kind of interface, > matching kmalloc? I guess returning an error code from memdup_user() lets callers distinguish between ENOMEM and EFAULT. Not sure if that's important or not but there probably are at least some sites that care. - R. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/6] xfs: use memdup_user() 2009-04-08 17:31 ` Roland Dreier @ 2009-04-09 0:43 ` Li Zefan 0 siblings, 0 replies; 5+ messages in thread From: Li Zefan @ 2009-04-09 0:43 UTC (permalink / raw) To: Roland Dreier; +Cc: Christoph Hellwig, Andrew Morton, LKML, xfs Roland Dreier wrote: > > wouldn't NULL be a better error return for this kind of interface, > > matching kmalloc? > > I guess returning an error code from memdup_user() lets callers > distinguish between ENOMEM and EFAULT. Not sure if that's important or > not but there probably are at least some sites that care. > Right, and this API is simlilar to strndup_user(). _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/6] xfs: use memdup_user() 2009-04-08 13:22 ` Christoph Hellwig 2009-04-08 17:31 ` Roland Dreier @ 2009-04-08 20:06 ` Felix Blyakher 1 sibling, 0 replies; 5+ messages in thread From: Felix Blyakher @ 2009-04-08 20:06 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Andrew Morton, Li Zefan, LKML, xfs On Apr 8, 2009, at 8:22 AM, Christoph Hellwig wrote: > On Wed, Apr 08, 2009 at 03:08:04PM +0800, Li Zefan wrote: >> Remove open-coded memdup_user() >> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> >> --- >> fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++---------------- >> fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++-------- >> 2 files changed, 11 insertions(+), 24 deletions(-) >> >> diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/ >> xfs_ioctl.c >> index d0b4994..34eaab6 100644 >> --- a/fs/xfs/linux-2.6/xfs_ioctl.c >> +++ b/fs/xfs/linux-2.6/xfs_ioctl.c >> @@ -489,17 +489,12 @@ xfs_attrmulti_attr_set( >> if (len > XATTR_SIZE_MAX) >> return EINVAL; >> >> - kbuf = kmalloc(len, GFP_KERNEL); >> - if (!kbuf) >> - return ENOMEM; >> - >> - if (copy_from_user(kbuf, ubuf, len)) >> - goto out_kfree; >> + kbuf = memdup_user(ubuf, len); >> + if (IS_ERR(kbuf)) >> + return PTR_ERR(kbuf); > > wouldn't NULL be a better error return for this kind of interface, > matching kmalloc? > > > Otherwise the patch looks good to me. > > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks good to me too. Reviewed-by: Felix Blyakher <felixb@sgi.com> p.s. Replying to reply as I couldn't find the original post. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-04-09 0:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <49DC4CC0.9050805@cn.fujitsu.com>
2009-04-08 7:08 ` [PATCH 4/6] xfs: use memdup_user() Li Zefan
2009-04-08 13:22 ` Christoph Hellwig
2009-04-08 17:31 ` Roland Dreier
2009-04-09 0:43 ` Li Zefan
2009-04-08 20:06 ` Felix Blyakher
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox