From: Dan Williams <dan.j.williams@intel.com>
Date: Thu, 31 Aug 2017 15:41:55 -0700
Subject: libnvdimm: fix integer overflow static analysis warning
Patch-mainline: v4.14-rc1
Git-commit: 58738c495e15badd2015e19ff41f1f1ed55200bc
References: FATE#323731
Dan reports:
The patch 62232e45f4a2: "libnvdimm: control (ioctl) messages for
nvdimm_bus and nvdimm devices" from Jun 8, 2015, leads to the
following static checker warning:
drivers/nvdimm/bus.c:1018 __nd_ioctl()
warn: integer overflows 'buf_len'
From a casual review, this seems like it might be a real bug. On
the first iteration we load some data into in_env[]. On the second
iteration we read a use controlled "in_size" from nd_cmd_in_size().
It can go up to UINT_MAX - 1. A high number means we will fill the
whole in_env[] buffer. But we potentially keep looping and adding
more to in_len so now it can be any value.
It simple enough to change, but it feels weird that we keep looping
even though in_env is totally full. Shouldn't we just return an
error if we don't have space for desc->in_num.
We keep looping because the size of the total input is allowed to be
bigger than the 'envelope' which is a subset of the payload that tells
us how much data to expect. For safety explicitly check that buf_len
does not overflow which is what the checker flagged.
Cc: <stable@vger.kernel.org>
Fixes: 62232e45f4a2: "libnvdimm: control (ioctl) messages for nvdimm_bus..."
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/nvdimm/bus.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -908,7 +908,6 @@ static int __nd_ioctl(struct nvdimm_bus
int read_only, unsigned int ioctl_cmd, unsigned long arg)
{
struct nvdimm_bus_descriptor *nd_desc = nvdimm_bus->nd_desc;
- size_t buf_len = 0, in_len = 0, out_len = 0;
static char out_env[ND_CMD_MAX_ENVELOPE];
static char in_env[ND_CMD_MAX_ENVELOPE];
const struct nd_cmd_desc *desc = NULL;
@@ -918,8 +917,10 @@ static int __nd_ioctl(struct nvdimm_bus
struct device *dev = &nvdimm_bus->dev;
struct nd_cmd_pkg pkg;
const char *cmd_name, *dimm_name;
+ u32 in_len = 0, out_len = 0;
unsigned long cmd_mask;
void *buf;
+ u64 buf_len = 0;
int rc, i, cmd_rc;
if (nvdimm) {
@@ -980,7 +981,7 @@ static int __nd_ioctl(struct nvdimm_bus
if (cmd == ND_CMD_CALL) {
func = pkg.nd_command;
- dev_dbg(dev, "%s:%s, idx: %llu, in: %zu, out: %zu, len %zu\n",
+ dev_dbg(dev, "%s:%s, idx: %llu, in: %u, out: %u, len %llu\n",
__func__, dimm_name, pkg.nd_command,
in_len, out_len, buf_len);
@@ -1010,9 +1011,9 @@ static int __nd_ioctl(struct nvdimm_bus
out_len += out_size;
}
- buf_len = out_len + in_len;
+ buf_len = (u64) out_len + (u64) in_len;
if (buf_len > ND_IOCTL_MAX_BUFLEN) {
- dev_dbg(dev, "%s:%s cmd: %s buf_len: %zu > %d\n", __func__,
+ dev_dbg(dev, "%s:%s cmd: %s buf_len: %llu > %d\n", __func__,
dimm_name, cmd_name, buf_len,
ND_IOCTL_MAX_BUFLEN);
return -EINVAL;