[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH 05/15] sheepdog: Fix snapshot ID parsing in _open(),
From: |
Markus Armbruster |
Subject: |
[Qemu-block] [PATCH 05/15] sheepdog: Fix snapshot ID parsing in _open(), _create, _goto() |
Date: |
Thu, 2 Mar 2017 22:43:56 +0100 |
sd_parse_uri() and sd_snapshot_goto() screw up error checking after
strtoul(), and truncate long tag names silently. Fix by replacing
those parts by new sd_parse_snapid_or_tag(), which checks more
carefully.
sd_snapshot_delete() also parses snapshot IDs, but is currently too
broken for me to touch. Mark TODO.
Two calls of strtol() without error checking remain in
parse_redundancy(). Mark them FIXME.
More silent truncation of configuration strings remains elsewhere.
Not marked.
Signed-off-by: Markus Armbruster <address@hidden>
---
block/sheepdog.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 55 insertions(+), 11 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 5554f47..deb110e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -914,6 +914,49 @@ static int get_sheep_fd(BDRVSheepdogState *s, Error **errp)
return fd;
}
+/*
+ * Parse numeric snapshot ID in @str
+ * If @str can't be parsed as number, return false.
+ * Else, if the number is zero or too large, set address@hidden to zero and
+ * return true.
+ * Else, set address@hidden to the number and return true.
+ */
+static bool sd_parse_snapid(const char *str, uint32_t *snapid)
+{
+ unsigned long ul;
+ int ret;
+
+ ret = qemu_strtoul(str, NULL, 10, &ul);
+ if (ret == -ERANGE) {
+ ul = ret = 0;
+ }
+ if (ret) {
+ return false;
+ }
+ if (ul > UINT32_MAX) {
+ ul = 0;
+ }
+
+ *snapid = ul;
+ return true;
+}
+
+static bool sd_parse_snapid_or_tag(const char *str,
+ uint32_t *snapid, char tag[])
+{
+ if (!sd_parse_snapid(str, snapid)) {
+ *snapid = 0;
+ if (g_strlcpy(tag, str, SD_MAX_VDI_TAG_LEN) >= SD_MAX_VDI_TAG_LEN) {
+ return false;
+ }
+ } else if (!*snapid) {
+ return false;
+ } else {
+ tag[0] = 0;
+ }
+ return true;
+}
+
static int sd_parse_uri(BDRVSheepdogState *s, const char *filename,
char *vdi, uint32_t *snapid, char *tag)
{
@@ -965,9 +1008,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const char
*filename,
/* snapshot tag */
if (uri->fragment) {
- *snapid = strtoul(uri->fragment, NULL, 10);
- if (*snapid == 0) {
- pstrcpy(tag, SD_MAX_VDI_TAG_LEN, uri->fragment);
+ if (!sd_parse_snapid_or_tag(uri->fragment, snapid, tag)) {
+ ret = -EINVAL;
+ goto out;
}
} else {
*snapid = CURRENT_VDI_ID; /* search current vdi */
@@ -1686,6 +1729,7 @@ static int parse_redundancy(BDRVSheepdogState *s, const
char *opt)
}
copy = strtol(n1, NULL, 10);
+ /* FIXME fix error checking by switching to qemu_strtol() */
if (copy > SD_MAX_COPIES || copy < 1) {
return -EINVAL;
}
@@ -1700,6 +1744,7 @@ static int parse_redundancy(BDRVSheepdogState *s, const
char *opt)
}
parity = strtol(n2, NULL, 10);
+ /* FIXME fix error checking by switching to qemu_strtol() */
if (parity >= SD_EC_MAX_STRIP || parity < 1) {
return -EINVAL;
}
@@ -2366,19 +2411,16 @@ static int sd_snapshot_goto(BlockDriverState *bs, const
char *snapshot_id)
BDRVSheepdogState *old_s;
char tag[SD_MAX_VDI_TAG_LEN];
uint32_t snapid = 0;
- int ret = 0;
+ int ret;
+
+ if (!sd_parse_snapid_or_tag(snapshot_id, &snapid, tag)) {
+ return -EINVAL;
+ }
old_s = g_new(BDRVSheepdogState, 1);
memcpy(old_s, s, sizeof(BDRVSheepdogState));
- snapid = strtoul(snapshot_id, NULL, 10);
- if (snapid) {
- tag[0] = 0;
- } else {
- pstrcpy(tag, sizeof(tag), snapshot_id);
- }
-
ret = reload_inode(s, snapid, tag);
if (ret) {
goto out;
@@ -2483,6 +2525,7 @@ static int sd_snapshot_delete(BlockDriverState *bs,
memset(buf, 0, sizeof(buf));
memset(snap_tag, 0, sizeof(snap_tag));
pstrcpy(buf, SD_MAX_VDI_LEN, s->name);
+ /* TODO Use sd_parse_snapid() once this mess is cleaned up */
ret = qemu_strtoul(snapshot_id, NULL, 10, &snap_id);
if (ret || snap_id > UINT32_MAX) {
/*
@@ -2499,6 +2542,7 @@ static int sd_snapshot_delete(BlockDriverState *bs,
hdr.snapid = (uint32_t) snap_id;
} else {
/* FIXME I suspect we should use @name here */
+ /* FIXME don't truncate silently */
pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id);
pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag);
}
--
2.7.4
[Qemu-block] [PATCH 02/15] sheepdog: Fix error handling in sd_snapshot_delete(), Markus Armbruster, 2017/03/02
[Qemu-block] [PATCH 05/15] sheepdog: Fix snapshot ID parsing in _open(), _create, _goto(),
Markus Armbruster <=
[Qemu-block] [PATCH 07/15] sheepdog: Report errors in pseudo-filename more usefully, Markus Armbruster, 2017/03/02
[Qemu-block] [PATCH 11/15] gluster: Don't duplicate qapi-util.c's qapi_enum_parse(), Markus Armbruster, 2017/03/02