2015-03-22 13:23:09

by Kinglong Mee

Subject: [PATCH] blkmapd: Make sure device root contains valid device id

When testing pnfs in virtual linux based on VirtualBox,
blkmapd gets dev_root->len == 0, which causes it Segmentation fault.

Signed-off-by: Kinglong Mee <[email protected]>
---
utils/blkmapd/device-inq.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
index eabc70c..c5bf71f 100644
--- a/utils/blkmapd/device-inq.c
+++ b/utils/blkmapd/device-inq.c
@@ -179,6 +179,7 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
char *buffer;
struct bl_dev_id *dev_root, *dev_id;
unsigned int pos, len, current_id = 0;
+ size_t devid_len = sizeof(struct bl_dev_id) - sizeof(unsigned char);

status = bldev_inquire_pages(fd, 0x83, &buffer);
if (status)
@@ -189,7 +190,11 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
pos = 0;
current_id = 0;
len = dev_root->len;
- while (pos < (len - sizeof(struct bl_dev_id) + sizeof(unsigned char))) {
+
+ if (len < devid_len)
+ goto out;
+
+ while (pos < (len - devid_len)) {
dev_id = (struct bl_dev_id *)&(dev_root->data[pos]);
if ((dev_id->ids & 0xf) < current_id)
continue;
@@ -221,8 +226,7 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
}
if (current_id == 3)
break;
- pos += (dev_id->len + sizeof(struct bl_dev_id) -
- sizeof(unsigned char));
+ pos += (dev_id->len + devid_len);
}
out:
if (!serial_out)
--
2.3.3

2015-03-25 08:42:37

by Christoph Hellwig

Subject: Re: [PATCH] blkmapd: Make sure device root contains valid device id

On Sun, Mar 22, 2015 at 09:22:59PM +0800, Kinglong Mee wrote:
> When testing pnfs in virtual linux based on VirtualBox,
> blkmapd gets dev_root->len == 0, which causes it Segmentation fault.

VirtualBox learly returns bogus values here, but it's always better to
be defensive, so:

Reviewed-by: Christoph Hellwig <[email protected]>

2015-03-26 15:29:55

by Steve Dickson

Subject: Re: [PATCH] blkmapd: Make sure device root contains valid device id

On 03/22/2015 09:22 AM, Kinglong Mee wrote:
> When testing pnfs in virtual linux based on VirtualBox,
> blkmapd gets dev_root->len == 0, which causes it Segmentation fault.
>
> Signed-off-by: Kinglong Mee <[email protected]>
Committed...

steved.

> ---
> utils/blkmapd/device-inq.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
> index eabc70c..c5bf71f 100644
> --- a/utils/blkmapd/device-inq.c
> +++ b/utils/blkmapd/device-inq.c
> @@ -179,6 +179,7 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
> char *buffer;
> struct bl_dev_id *dev_root, *dev_id;
> unsigned int pos, len, current_id = 0;
> + size_t devid_len = sizeof(struct bl_dev_id) - sizeof(unsigned char);
>
> status = bldev_inquire_pages(fd, 0x83, &buffer);
> if (status)
> @@ -189,7 +190,11 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
> pos = 0;
> current_id = 0;
> len = dev_root->len;
> - while (pos < (len - sizeof(struct bl_dev_id) + sizeof(unsigned char))) {
> +
> + if (len < devid_len)
> + goto out;
> +
> + while (pos < (len - devid_len)) {
> dev_id = (struct bl_dev_id *)&(dev_root->data[pos]);
> if ((dev_id->ids & 0xf) < current_id)
> continue;
> @@ -221,8 +226,7 @@ struct bl_serial *bldev_read_serial(int fd, const char *filename)
> }
> if (current_id == 3)
> break;
> - pos += (dev_id->len + sizeof(struct bl_dev_id) -
> - sizeof(unsigned char));
> + pos += (dev_id->len + devid_len);
> }
> out:
> if (!serial_out)
>