[2/6] vfs: cap dedupe request structure size at PAGE_SIZE
diff mbox

Message ID 147216785429.525.1965983896010934181.stgit@birch.djwong.org
State Not Applicable
Headers show

Commit Message

Darrick J. Wong Aug. 25, 2016, 11:30 p.m. UTC
Kirill A. Shutemov reports that the kernel doesn't try to cap dest_count
in any way, and uses the number to allocate kernel memory.  This causes
high order allocation warnings in the kernel log if someone passes in a
big enough value.  We should clamp the allocation at PAGE_SIZE to avoid
stressing the VM.

The two existing users of the dedupe ioctl never send more than 120
requests, so we can safely clamp dest_range at PAGE_SIZE, because with
4k pages we can handle up to 127 dedupe candidates.  Given the max
extent length of 16MB, we can end up doing 2GB of IO which is plenty.

Reported-by: "Kirill A. Shutemov" <kirill@shutemov.name>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/ioctl.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Christoph Hellwig Sept. 5, 2016, 2:55 p.m. UTC | #1
This really should go into 4.8 (and the previous patch probably as
well), and I've said that a couple of times, and you tried a few times
to send it to Al at least.

Al, do you want to pick it up or should Darrick send it straight to
Linus?

On Thu, Aug 25, 2016 at 04:30:54PM -0700, Darrick J. Wong wrote:
> Kirill A. Shutemov reports that the kernel doesn't try to cap dest_count
> in any way, and uses the number to allocate kernel memory.  This causes
> high order allocation warnings in the kernel log if someone passes in a
> big enough value.  We should clamp the allocation at PAGE_SIZE to avoid
> stressing the VM.
> 
> The two existing users of the dedupe ioctl never send more than 120
> requests, so we can safely clamp dest_range at PAGE_SIZE, because with
> 4k pages we can handle up to 127 dedupe candidates.  Given the max
> extent length of 16MB, we can end up doing 2GB of IO which is plenty.
> 
> Reported-by: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/ioctl.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 26aba09..c415668 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -582,6 +582,10 @@ static int ioctl_file_dedupe_range(struct file *file, void __user *arg)
>  	}
>  
>  	size = offsetof(struct file_dedupe_range __user, info[count]);
> +	if (size > PAGE_SIZE) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  
>  	same = memdup_user(argp, size);
>  	if (IS_ERR(same)) {
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
Darrick J. Wong Sept. 13, 2016, 1:16 a.m. UTC | #2
On Mon, Sep 05, 2016 at 07:55:49AM -0700, Christoph Hellwig wrote:
> This really should go into 4.8 (and the previous patch probably as
> well), and I've said that a couple of times, and you tried a few times
> to send it to Al at least.
> 
> Al, do you want to pick it up or should Darrick send it straight to
> Linus?

Ping?  Anyone?

--D

> On Thu, Aug 25, 2016 at 04:30:54PM -0700, Darrick J. Wong wrote:
> > Kirill A. Shutemov reports that the kernel doesn't try to cap dest_count
> > in any way, and uses the number to allocate kernel memory.  This causes
> > high order allocation warnings in the kernel log if someone passes in a
> > big enough value.  We should clamp the allocation at PAGE_SIZE to avoid
> > stressing the VM.
> > 
> > The two existing users of the dedupe ioctl never send more than 120
> > requests, so we can safely clamp dest_range at PAGE_SIZE, because with
> > 4k pages we can handle up to 127 dedupe candidates.  Given the max
> > extent length of 16MB, we can end up doing 2GB of IO which is plenty.
> > 
> > Reported-by: "Kirill A. Shutemov" <kirill@shutemov.name>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/ioctl.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > 
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 26aba09..c415668 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -582,6 +582,10 @@ static int ioctl_file_dedupe_range(struct file *file, void __user *arg)
> >  	}
> >  
> >  	size = offsetof(struct file_dedupe_range __user, info[count]);
> > +	if (size > PAGE_SIZE) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> >  
> >  	same = memdup_user(argp, size);
> >  	if (IS_ERR(same)) {
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> ---end quoted text---
Christoph Hellwig Sept. 14, 2016, 8:22 p.m. UTC | #3
On Mon, Sep 12, 2016 at 06:16:20PM -0700, Darrick J. Wong wrote:
> Ping?  Anyone?

Please resend the patch straight to Linux with a Cc to Al and me.

Patch
diff mbox

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 26aba09..c415668 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -582,6 +582,10 @@  static int ioctl_file_dedupe_range(struct file *file, void __user *arg)
 	}
 
 	size = offsetof(struct file_dedupe_range __user, info[count]);
+	if (size > PAGE_SIZE) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	same = memdup_user(argp, size);
 	if (IS_ERR(same)) {