* [patch] NVMe: check for integer overflow in nvme_map_user_pages()
@ 2013-05-11 16:31 Dan Carpenter
2013-05-13 13:53 ` Wilcox, Matthew R
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2013-05-11 16:31 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: Keith Busch, Vishal Verma, linux-kernel, kernel-janitors
You need to have CAP_SYS_ADMIN to trigger this overflow but it makes the
static checkers complain so we should fix it. The worry is that
"length" comes from copy_from_user() so we need to check that "length +
offset" can't overflow.
I also changed the min_t() cast to be unsigned instead of signed. Now
that we cap "length" to INT_MAX it doesn't make a difference, but it's a
little easier for reviewers to know that large values aren't cast to
negative.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 8efdfaa..4376375 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1206,7 +1206,7 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
if (addr & 3)
return ERR_PTR(-EINVAL);
- if (!length)
+ if (!length || length > INT_MAX - PAGE_SIZE)
return ERR_PTR(-EINVAL);
offset = offset_in_page(addr);
@@ -1227,7 +1227,8 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
sg_init_table(sg, count);
for (i = 0; i < count; i++) {
sg_set_page(&sg[i], pages[i],
- min_t(int, length, PAGE_SIZE - offset), offset);
+ min_t(unsigned, length, PAGE_SIZE - offset),
+ offset);
length -= (PAGE_SIZE - offset);
offset = 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [patch] NVMe: check for integer overflow in nvme_map_user_pages()
2013-05-11 16:31 [patch] NVMe: check for integer overflow in nvme_map_user_pages() Dan Carpenter
@ 2013-05-13 13:53 ` Wilcox, Matthew R
2013-05-13 14:59 ` [patch -resend] " Dan Carpenter
2013-05-13 15:00 ` [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Wilcox, Matthew R @ 2013-05-13 13:53 UTC (permalink / raw)
To: Dan Carpenter
Cc: Busch, Keith, Verma, Vishal L, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Why didn't you send this patch to the linux-nvme mailing list or the address listed in MAINTAINERS? Now Exchange & Outlook have hold of it, and it's mangled.
NVM EXPRESS DRIVER
M: Matthew Wilcox <willy@linux.intel.com>
L: linux-nvme@lists.infradead.org
________________________________________
From: Dan Carpenter [dan.carpenter@oracle.com]
Sent: May 11, 2013 9:31 AM
To: Wilcox, Matthew R
Cc: Busch, Keith; Verma, Vishal L; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
Subject: [patch] NVMe: check for integer overflow in nvme_map_user_pages()
You need to have CAP_SYS_ADMIN to trigger this overflow but it makes the
static checkers complain so we should fix it. The worry is that
"length" comes from copy_from_user() so we need to check that "length +
offset" can't overflow.
I also changed the min_t() cast to be unsigned instead of signed. Now
that we cap "length" to INT_MAX it doesn't make a difference, but it's a
little easier for reviewers to know that large values aren't cast to
negative.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 8efdfaa..4376375 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1206,7 +1206,7 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
if (addr & 3)
return ERR_PTR(-EINVAL);
- if (!length)
+ if (!length || length > INT_MAX - PAGE_SIZE)
return ERR_PTR(-EINVAL);
offset = offset_in_page(addr);
@@ -1227,7 +1227,8 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
sg_init_table(sg, count);
for (i = 0; i < count; i++) {
sg_set_page(&sg[i], pages[i],
- min_t(int, length, PAGE_SIZE - offset), offset);
+ min_t(unsigned, length, PAGE_SIZE - offset),
+ offset);
length -= (PAGE_SIZE - offset);
offset = 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch -resend] NVMe: check for integer overflow in nvme_map_user_pages()
2013-05-13 13:53 ` Wilcox, Matthew R
@ 2013-05-13 14:59 ` Dan Carpenter
2013-05-17 13:41 ` Matthew Wilcox
2013-05-13 15:00 ` [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2013-05-13 14:59 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Keith Busch, Vishal Verma, linux-nvme, linux-kernel,
kernel-janitors
You need to have CAP_SYS_ADMIN to trigger this overflow but it makes the
static checkers complain so we should fix it. The worry is that
"length" comes from copy_from_user() so we need to check that "length +
offset" can't overflow.
I also changed the min_t() cast to be unsigned instead of signed. Now
that we cap "length" to INT_MAX it doesn't make a difference, but it's a
little easier for reviewers to know that large values aren't cast to
negative.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Resending to the linux-nvme@lists.infradead.org email address.
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 8efdfaa..4376375 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1206,7 +1206,7 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
if (addr & 3)
return ERR_PTR(-EINVAL);
- if (!length)
+ if (!length || length > INT_MAX - PAGE_SIZE)
return ERR_PTR(-EINVAL);
offset = offset_in_page(addr);
@@ -1227,7 +1227,8 @@ struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
sg_init_table(sg, count);
for (i = 0; i < count; i++) {
sg_set_page(&sg[i], pages[i],
- min_t(int, length, PAGE_SIZE - offset), offset);
+ min_t(unsigned, length, PAGE_SIZE - offset),
+ offset);
length -= (PAGE_SIZE - offset);
offset = 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list
2013-05-13 13:53 ` Wilcox, Matthew R
2013-05-13 14:59 ` [patch -resend] " Dan Carpenter
@ 2013-05-13 15:00 ` Dan Carpenter
2013-05-17 13:41 ` Matthew Wilcox
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2013-05-13 15:00 UTC (permalink / raw)
To: Wilcox, Matthew R
Cc: Busch, Keith, Verma, Vishal L, linux-nvme,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
There isn't an nvme.c file any more. It has been split into multiple
files.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 8f190f5..97ec343 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5747,7 +5747,7 @@ M: Matthew Wilcox <willy@linux.intel.com>
L: linux-nvme@lists.infradead.org
T: git git://git.infradead.org/users/willy/linux-nvme.git
S: Supported
-F: drivers/block/nvme.c
+F: drivers/block/nvme*
F: include/linux/nvme.h
OMAP SUPPORT
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list
2013-05-13 15:00 ` [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list Dan Carpenter
@ 2013-05-17 13:41 ` Matthew Wilcox
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2013-05-17 13:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Wilcox, Matthew R, Busch, Keith, Verma, Vishal L,
kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-nvme
On Mon, May 13, 2013 at 06:00:42PM +0300, Dan Carpenter wrote:
> There isn't an nvme.c file any more. It has been split into multiple
> files.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch -resend] NVMe: check for integer overflow in nvme_map_user_pages()
2013-05-13 14:59 ` [patch -resend] " Dan Carpenter
@ 2013-05-17 13:41 ` Matthew Wilcox
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2013-05-17 13:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Matthew Wilcox, Keith Busch, Vishal Verma, kernel-janitors,
linux-kernel, linux-nvme
On Mon, May 13, 2013 at 05:59:50PM +0300, Dan Carpenter wrote:
> You need to have CAP_SYS_ADMIN to trigger this overflow but it makes the
> static checkers complain so we should fix it. The worry is that
> "length" comes from copy_from_user() so we need to check that "length +
> offset" can't overflow.
>
> I also changed the min_t() cast to be unsigned instead of signed. Now
> that we cap "length" to INT_MAX it doesn't make a difference, but it's a
> little easier for reviewers to know that large values aren't cast to
> negative.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-17 13:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-11 16:31 [patch] NVMe: check for integer overflow in nvme_map_user_pages() Dan Carpenter
2013-05-13 13:53 ` Wilcox, Matthew R
2013-05-13 14:59 ` [patch -resend] " Dan Carpenter
2013-05-17 13:41 ` Matthew Wilcox
2013-05-13 15:00 ` [patch] MAINTAINERS: update NVM EXPRESS DRIVER file list Dan Carpenter
2013-05-17 13:41 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox