* [patch 1/3] xen/privcmd: check for integer overflow in ioctl
@ 2012-09-08 9:52 Dan Carpenter
2012-09-09 19:49 ` Andres Lagar-Cavilla
2012-09-10 10:35 ` David Vrabel
0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2012-09-08 9:52 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: kernel-janitors, Jeremy Fitzhardinge, xen-devel, virtualization,
Konrad Rzeszutek Wilk
If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication
could overflow and the access_ok() check wouldn't test the right size.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Only needed in linux-next.
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 215a3c0..fdff8f9 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -325,6 +325,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
return -EFAULT;
/* Returns per-frame error in m.arr. */
m.err = NULL;
+ if (m.num > SIZE_MAX / sizeof(*m.arr))
+ return -EINVAL;
if (!access_ok(VERIFY_WRITE, m.arr, m.num * sizeof(*m.arr)))
return -EFAULT;
break;
@@ -332,6 +334,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
if (copy_from_user(&m, udata, sizeof(struct privcmd_mmapbatch_v2)))
return -EFAULT;
/* Returns per-frame error code in m.err. */
+ if (m.num > SIZE_MAX / sizeof(*m.err))
+ return -EINVAL;
if (!access_ok(VERIFY_WRITE, m.err, m.num * (sizeof(*m.err))))
return -EFAULT;
break;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [patch 1/3] xen/privcmd: check for integer overflow in ioctl
2012-09-08 9:52 [patch 1/3] xen/privcmd: check for integer overflow in ioctl Dan Carpenter
@ 2012-09-09 19:49 ` Andres Lagar-Cavilla
2012-09-10 10:35 ` David Vrabel
1 sibling, 0 replies; 4+ messages in thread
From: Andres Lagar-Cavilla @ 2012-09-09 19:49 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jeremy Fitzhardinge, xen-devel, Konrad Rzeszutek Wilk,
Andres Lagar-Cavilla, kernel-janitors, virtualization
On Sep 8, 2012, at 5:52 AM, Dan Carpenter wrote:
> If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication
> could overflow and the access_ok() check wouldn't test the right size.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> ---
> Only needed in linux-next.
>
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 215a3c0..fdff8f9 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -325,6 +325,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> return -EFAULT;
> /* Returns per-frame error in m.arr. */
> m.err = NULL;
> + if (m.num > SIZE_MAX / sizeof(*m.arr))
> + return -EINVAL;
> if (!access_ok(VERIFY_WRITE, m.arr, m.num * sizeof(*m.arr)))
> return -EFAULT;
> break;
> @@ -332,6 +334,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> if (copy_from_user(&m, udata, sizeof(struct privcmd_mmapbatch_v2)))
> return -EFAULT;
> /* Returns per-frame error code in m.err. */
> + if (m.num > SIZE_MAX / sizeof(*m.err))
> + return -EINVAL;
> if (!access_ok(VERIFY_WRITE, m.err, m.num * (sizeof(*m.err))))
> return -EFAULT;
> break;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 1/3] xen/privcmd: check for integer overflow in ioctl
2012-09-08 9:52 [patch 1/3] xen/privcmd: check for integer overflow in ioctl Dan Carpenter
2012-09-09 19:49 ` Andres Lagar-Cavilla
@ 2012-09-10 10:35 ` David Vrabel
2012-09-10 11:20 ` [Xen-devel] " Dan Carpenter
1 sibling, 1 reply; 4+ messages in thread
From: David Vrabel @ 2012-09-10 10:35 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jeremy Fitzhardinge, xen-devel, Konrad Rzeszutek Wilk,
Andres Lagar-Cavilla, kernel-janitors, virtualization
On 08/09/12 10:52, Dan Carpenter wrote:
> If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication
> could overflow and the access_ok() check wouldn't test the right size.
m.num is range checked later on so it doesn't matter that the
access_ok() checks might be wrong. A bit subtle, perhaps.
David
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Only needed in linux-next.
>
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 215a3c0..fdff8f9 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -325,6 +325,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> return -EFAULT;
> /* Returns per-frame error in m.arr. */
> m.err = NULL;
> + if (m.num > SIZE_MAX / sizeof(*m.arr))
> + return -EINVAL;
> if (!access_ok(VERIFY_WRITE, m.arr, m.num * sizeof(*m.arr)))
> return -EFAULT;
> break;
> @@ -332,6 +334,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> if (copy_from_user(&m, udata, sizeof(struct privcmd_mmapbatch_v2)))
> return -EFAULT;
> /* Returns per-frame error code in m.err. */
> + if (m.num > SIZE_MAX / sizeof(*m.err))
> + return -EINVAL;
> if (!access_ok(VERIFY_WRITE, m.err, m.num * (sizeof(*m.err))))
> return -EFAULT;
> break;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [patch 1/3] xen/privcmd: check for integer overflow in ioctl
2012-09-10 10:35 ` David Vrabel
@ 2012-09-10 11:20 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2012-09-10 11:20 UTC (permalink / raw)
To: David Vrabel
Cc: Jeremy Fitzhardinge, xen-devel, Konrad Rzeszutek Wilk,
Andres Lagar-Cavilla, kernel-janitors, virtualization
On Mon, Sep 10, 2012 at 11:35:11AM +0100, David Vrabel wrote:
> On 08/09/12 10:52, Dan Carpenter wrote:
> > If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication
> > could overflow and the access_ok() check wouldn't test the right size.
>
> m.num is range checked later on so it doesn't matter that the
> access_ok() checks might be wrong. A bit subtle, perhaps.
>
Yeah. It's too subtle for my static checker but not so subtle for
a human being. Laziness on my part.
Please drop this patch.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-10 11:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-08 9:52 [patch 1/3] xen/privcmd: check for integer overflow in ioctl Dan Carpenter
2012-09-09 19:49 ` Andres Lagar-Cavilla
2012-09-10 10:35 ` David Vrabel
2012-09-10 11:20 ` [Xen-devel] " Dan Carpenter
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).