linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE
@ 2016-07-28 18:35 Darrick J. Wong
  2016-08-01  6:31 ` Christoph Hellwig
  2016-08-02  2:35 ` Mark Fasheh
  0 siblings, 2 replies; 4+ messages in thread
From: Darrick J. Wong @ 2016-07-28 18:35 UTC (permalink / raw)
  To: Al Viro
  Cc: david, linux-fsdevel, linux-api, xfs, Vlastimil Babka,
	Kirill A. Shutemov

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 db3d033..57409a7 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)) {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE
  2016-07-28 18:35 [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE Darrick J. Wong
@ 2016-08-01  6:31 ` Christoph Hellwig
  2016-08-02  2:35 ` Mark Fasheh
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2016-08-01  6:31 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Al Viro, linux-api, xfs, linux-fsdevel, Kirill A. Shutemov,
	Vlastimil Babka

On Thu, Jul 28, 2016 at 11:35:34AM -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.

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

> @@ -582,6 +582,10 @@ static int ioctl_file_dedupe_range(struct file *file, void __user *arg)

This function returns long in mainline.  Maybe you should resend your
return type fix to Al while you're at it?


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE
  2016-07-28 18:35 [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE Darrick J. Wong
  2016-08-01  6:31 ` Christoph Hellwig
@ 2016-08-02  2:35 ` Mark Fasheh
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Fasheh @ 2016-08-02  2:35 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Al Viro, linux-api, xfs, linux-fsdevel, Kirill A. Shutemov,
	Vlastimil Babka

On Thu, Jul 28, 2016 at 11:35:34AM -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>

Reviewed-by: Mark Fasheh <mfasheh@suse.de>

--
Mark Fasheh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE
@ 2016-09-15  3:20 Darrick J. Wong
  0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2016-09-15  3:20 UTC (permalink / raw)
  To: torvalds
  Cc: Christoph Hellwig, david, viro, linux-xfs, Kirill A. Shutemov,
	xfs, linux-fsdevel, linux-api

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)) {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-09-15  3:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 18:35 [PATCH] vfs: cap dedupe request structure size at PAGE_SIZE Darrick J. Wong
2016-08-01  6:31 ` Christoph Hellwig
2016-08-02  2:35 ` Mark Fasheh
2016-09-15  3:20 Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).