* [PATCH] Add data transfer length check for admin commands
@ 2013-09-02 17:00 Prasad Joshi
0 siblings, 0 replies; 4+ messages in thread
From: Prasad Joshi @ 2013-09-02 17:00 UTC (permalink / raw)
From: Prasad Joshi <pjoshi@stec-inc.com>
According to NVM Express 1.1 specifications, the lower 2 bits of a
NVME command opcode indicates, the data transfer (Figure 38). Zero value
of these two bits indicates, data length in actual NVME command is not
required. Similarly non-zero value indicates mandatory data transfer
length. The patch adds a verification of these bits along with correct
value of data transfer length.
Suggested-by: Matthew Wilcox <willy at linux.intel.com>
Signed-off-by: Prasad Joshi <pjoshi at stec-inc.com>
Signed-off-by: Anup Shendkar <ashenkar at stec-inc.com>
---
drivers/block/nvme-core.c | 3 +++
include/linux/nvme.h | 7 +++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ce79a59..256278e 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1402,6 +1402,9 @@ static int nvme_user_admin_cmd(struct nvme_dev *dev,
return -EACCES;
if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
return -EFAULT;
+ if (((cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && !cmd.data_len) ||
+ (!(cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && cmd.data_len))
+ return -EINVAL;
memset(&c, 0, sizeof(c));
c.common.opcode = cmd.opcode;
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index f451c8d..3b2c8ee 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -508,6 +508,13 @@ struct nvme_admin_cmd {
#define NVME_IOCTL_ADMIN_CMD _IOWR('N', 0x41, struct nvme_admin_cmd)
#define NVME_IOCTL_SUBMIT_IO _IOW('N', 0x42, struct nvme_user_io)
+/*
+ * The 2 LSB bits of NVME Admin command opcode are called as data transfer bits.
+ * These two bits define where a command should include data transfer
+ * information.
+ */
+#define NVME_ADMIN_CMD_DATA_XFER_MASK (0b11)
+
#ifdef __KERNEL__
#include <linux/pci.h>
#include <linux/miscdevice.h>
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] Add data transfer length check for admin commands
@ 2013-09-02 17:01 Prasad Joshi
2013-09-03 17:05 ` Matthew Wilcox
0 siblings, 1 reply; 4+ messages in thread
From: Prasad Joshi @ 2013-09-02 17:01 UTC (permalink / raw)
From: Prasad Joshi <pjoshi@stec-inc.com>
According to NVM Express 1.1 specifications, the lower 2 bits of a
NVME command opcode indicates, the data transfer (Figure 38). Zero value
of these two bits indicates, data length in actual NVME command is not
required. Similarly non-zero value indicates mandatory data transfer
length. The patch adds a verification of these bits along with correct
value of data transfer length.
Suggested-by: Matthew Wilcox <willy at linux.intel.com>
Signed-off-by: Prasad Joshi <pjoshi at stec-inc.com>
Signed-off-by: Anup Shendkar <ashendkar at stec-inc.com>
---
drivers/block/nvme-core.c | 3 +++
include/linux/nvme.h | 7 +++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ce79a59..256278e 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1402,6 +1402,9 @@ static int nvme_user_admin_cmd(struct nvme_dev *dev,
return -EACCES;
if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
return -EFAULT;
+ if (((cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && !cmd.data_len) ||
+ (!(cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && cmd.data_len))
+ return -EINVAL;
memset(&c, 0, sizeof(c));
c.common.opcode = cmd.opcode;
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index f451c8d..3b2c8ee 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -508,6 +508,13 @@ struct nvme_admin_cmd {
#define NVME_IOCTL_ADMIN_CMD _IOWR('N', 0x41, struct nvme_admin_cmd)
#define NVME_IOCTL_SUBMIT_IO _IOW('N', 0x42, struct nvme_user_io)
+/*
+ * The 2 LSB bits of NVME Admin command opcode are called as data transfer bits.
+ * These two bits define where a command should include data transfer
+ * information.
+ */
+#define NVME_ADMIN_CMD_DATA_XFER_MASK (0b11)
+
#ifdef __KERNEL__
#include <linux/pci.h>
#include <linux/miscdevice.h>
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] Add data transfer length check for admin commands
2013-09-02 17:01 Prasad Joshi
@ 2013-09-03 17:05 ` Matthew Wilcox
0 siblings, 0 replies; 4+ messages in thread
From: Matthew Wilcox @ 2013-09-03 17:05 UTC (permalink / raw)
On Mon, Sep 02, 2013@10:31:16PM +0530, Prasad Joshi wrote:
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
I think you generated this patch against Linus' tree rather than the NVMe
tree (http://git.infradead.org/users/willy/linux-nvme.git). The __KERNEL__
ifdefs are gone due to the separation of this file into the uapi and non-uapi
versions.
> +/*
> + * The 2 LSB bits of NVME Admin command opcode are called as data transfer bits.
> + * These two bits define where a command should include data transfer
> + * information.
> + */
> +#define NVME_ADMIN_CMD_DATA_XFER_MASK (0b11)
> +
Binary constants are a GCC extension, which I would prefer to avoid.
They're only available from gcc 4.3 onwards, and apparently we still
support gcc versions as old as 3.2.
The bits aren't just for admin commands (so we should correct both the
comment and the name of the constant). How about adding:
NVME_DATA_XFER_NONE = 0,
NVME_DATA_XFER_READ = 1,
NVME_DATA_XFER_WRITE = 2,
NVME_DATA_XFER_BIDI = 3,
NVME_DATA_XFER_MASK = 3,
to the 'enum nvme_opcode' definition?
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Add data transfer length check for admin commands
@ 2013-09-02 16:59 Prasad Joshi
0 siblings, 0 replies; 4+ messages in thread
From: Prasad Joshi @ 2013-09-02 16:59 UTC (permalink / raw)
From: Prasad Joshi <pjoshi@stec-inc.com>
According to NVM Express 1.1 specifications, the lower 2 bits of a
NVME command opcode indicates, the data transfer (Figure 38). Zero value
of these two bits indicates, data length in actual NVME command is not
required. Similarly non-zero value indicates mandatory data transfer
length. The patch adds a verification of these bits along with correct
value of data transfer length.
Suggested-by: Matthew Wilcox <willy at linux.intel.com>
Signed-off-by: Prasad Joshi <pjoshi at stec-inc.com>
Signed-off-by: Anup Shendkar <ashenkar at stec-inc.com>
---
drivers/block/nvme-core.c | 3 +++
include/linux/nvme.h | 7 +++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ce79a59..256278e 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1402,6 +1402,9 @@ static int nvme_user_admin_cmd(struct nvme_dev *dev,
return -EACCES;
if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
return -EFAULT;
+ if (((cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && !cmd.data_len) ||
+ (!(cmd.opcode & NVME_ADMIN_CMD_DATA_XFER_MASK) && cmd.data_len))
+ return -EINVAL;
memset(&c, 0, sizeof(c));
c.common.opcode = cmd.opcode;
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index f451c8d..3b2c8ee 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -508,6 +508,13 @@ struct nvme_admin_cmd {
#define NVME_IOCTL_ADMIN_CMD _IOWR('N', 0x41, struct nvme_admin_cmd)
#define NVME_IOCTL_SUBMIT_IO _IOW('N', 0x42, struct nvme_user_io)
+/*
+ * The 2 LSB bits of NVME Admin command opcode are called as data transfer bits.
+ * These two bits define where a command should include data transfer
+ * information.
+ */
+#define NVME_ADMIN_CMD_DATA_XFER_MASK (0b11)
+
#ifdef __KERNEL__
#include <linux/pci.h>
#include <linux/miscdevice.h>
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-03 17:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-02 17:00 [PATCH] Add data transfer length check for admin commands Prasad Joshi
-- strict thread matches above, loose matches on Subject: below --
2013-09-02 17:01 Prasad Joshi
2013-09-03 17:05 ` Matthew Wilcox
2013-09-02 16:59 Prasad Joshi
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).