[libvirt] [PATCH for 1.2.10 0/2] Fix crash when parsing backing chain URI with transport
See patch 1 for explanation. Peter Krempa (2): storage: Fix crash when parsing backing store URI with schema test: Add test to verify helpers used for backing file name parsing src/util/virstoragefile.c | 10 +++++----- tests/virstoragetest.c | 31 +++++++++++++++++++++++++------ tests/virstringtest.c | 3 +++ tests/viruritest.c | 1 + 4 files changed, 34 insertions(+), 11 deletions(-) -- 2.1.0
The code that parses the schema from the URI touches the "hosts[0]" member of the storage file source structure in case the URI contains a schema. The hosts array was not yet allocated at the point in the code where the transport protocol was parsed and set. This lead to a crash of libvirtd. Fix the code by allocating the "hosts" array upfront and add a test case to verify this scenario. (Unfortunately this requires shuffling the test case numbers too). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1156288 --- src/util/virstoragefile.c | 10 +++++----- tests/virstoragetest.c | 31 +++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 960aa23..8e9d115 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2133,6 +2133,11 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src, goto cleanup; } + if (VIR_ALLOC(src->hosts) < 0) + goto cleanup; + + src->nhosts = 1; + if (!(scheme = virStringSplit(uri->scheme, "+", 2))) goto cleanup; @@ -2183,11 +2188,6 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src, tmp[0] = '\0'; } - if (VIR_ALLOC(src->hosts) < 0) - goto cleanup; - - src->nhosts = 1; - if (uri->port > 0) { if (virAsprintf(&src->hosts->port, "%d", uri->port) < 0) goto cleanup; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 29f5c7a..05e48f3 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -835,6 +835,25 @@ mymain(void) (&qcow2, &nbd), EXP_PASS, (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS); + /* Rewrite qcow2 to use an nbd: protocol as backend */ + virCommandFree(cmd); + cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2", + "-F", "raw", "-b", "nbd+tcp://example.org:6000/blah", + "qcow2", NULL); + if (virCommandRun(cmd, NULL) < 0) + ret = -1; + qcow2.expBackingStoreRaw = "nbd+tcp://example.org:6000/blah"; + + /* Qcow2 file with backing protocol instead of file */ + testFileData nbd2 = { + .path = "blah", + .type = VIR_STORAGE_TYPE_NETWORK, + .format = VIR_STORAGE_FILE_RAW, + }; + TEST_CHAIN(12, absqcow2, VIR_STORAGE_FILE_QCOW2, + (&qcow2, &nbd2), EXP_PASS, + (&qcow2, &nbd2), ALLOW_PROBE | EXP_PASS); + /* qed file */ testFileData qed = { .expBackingStoreRaw = absraw, @@ -848,7 +867,7 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(12, absqed, VIR_STORAGE_FILE_AUTO, + TEST_CHAIN(13, absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS, (&qed, &raw), ALLOW_PROBE | EXP_PASS); @@ -858,10 +877,10 @@ mymain(void) .type = VIR_STORAGE_TYPE_DIR, .format = VIR_STORAGE_FILE_DIR, }; - TEST_CHAIN(13, absdir, VIR_STORAGE_FILE_AUTO, + TEST_CHAIN(14, absdir, VIR_STORAGE_FILE_AUTO, (&dir), EXP_PASS, (&dir), ALLOW_PROBE | EXP_PASS); - TEST_CHAIN(14, absdir, VIR_STORAGE_FILE_DIR, + TEST_CHAIN(15, absdir, VIR_STORAGE_FILE_DIR, (&dir), EXP_PASS, (&dir), ALLOW_PROBE | EXP_PASS); @@ -900,7 +919,7 @@ mymain(void) raw.path = datadir "/sub/../sub/../raw"; raw.pathRel = "../raw"; - TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2, + TEST_CHAIN(16, abslink2, VIR_STORAGE_FILE_QCOW2, (&link2, &link1, &raw), EXP_PASS, (&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS); #endif @@ -914,7 +933,7 @@ mymain(void) qcow2.expBackingStoreRaw = "qcow2"; /* Behavior of an infinite loop chain */ - TEST_CHAIN(16, absqcow2, VIR_STORAGE_FILE_QCOW2, + TEST_CHAIN(17, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, (&qcow2), ALLOW_PROBE | EXP_WARN); @@ -933,7 +952,7 @@ mymain(void) qcow2.expBackingStoreRaw = "wrap"; /* Behavior of an infinite loop chain */ - TEST_CHAIN(17, abswrap, VIR_STORAGE_FILE_QCOW2, + TEST_CHAIN(18, abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_WARN, (&wrap, &qcow2), ALLOW_PROBE | EXP_WARN); -- 2.1.0
Add two test cases to verify that the helpers split and parse the backing store components properly. --- tests/virstringtest.c | 3 +++ tests/viruritest.c | 1 + 2 files changed, 4 insertions(+) diff --git a/tests/virstringtest.c b/tests/virstringtest.c index 841531f..a0bfd61 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -597,6 +597,9 @@ mymain(void) const char *tokens7[] = { "The", "quick", "brown", "fox", "", NULL }; TEST_SPLIT("The quick brown fox ", " ", 0, tokens7); + const char *tokens8[] = { "gluster", "rdma", NULL }; + TEST_SPLIT("gluster+rdma", "+", 2, tokens8); + if (virtTestRun("strdup", testStrdup, NULL) < 0) ret = -1; diff --git a/tests/viruritest.c b/tests/viruritest.c index dbcd877..48b5865 100644 --- a/tests/viruritest.c +++ b/tests/viruritest.c @@ -173,6 +173,7 @@ mymain(void) TEST_PARSE("test://127.0.0.1:123/system", "test", "127.0.0.1", 123, "/system", NULL, NULL, NULL, NULL); TEST_PARSE("test://[::1]:123/system", "test", "::1", 123, "/system", NULL, NULL, NULL, NULL); TEST_PARSE("test://[2001:41c8:1:4fd4::2]:123/system", "test", "2001:41c8:1:4fd4::2", 123, "/system", NULL, NULL, NULL, NULL); + TEST_PARSE("gluster+rdma://example.com:1234/gv0/vol.img", "gluster+rdma", "example.com", 1234, "/gv0/vol.img", NULL, NULL, NULL, NULL); virURIParam params1[] = { { (char*)"foo", (char*)"one", false }, -- 2.1.0
On 10/29/2014 04:12 AM, Peter Krempa wrote:
See patch 1 for explanation.
Peter Krempa (2): storage: Fix crash when parsing backing store URI with schema test: Add test to verify helpers used for backing file name parsing
src/util/virstoragefile.c | 10 +++++----- tests/virstoragetest.c | 31 +++++++++++++++++++++++++------ tests/virstringtest.c | 3 +++ tests/viruritest.c | 1 + 4 files changed, 34 insertions(+), 11 deletions(-)
ACK series
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/29/14 16:50, Eric Blake wrote:
On 10/29/2014 04:12 AM, Peter Krempa wrote:
See patch 1 for explanation.
Peter Krempa (2): storage: Fix crash when parsing backing store URI with schema test: Add test to verify helpers used for backing file name parsing
src/util/virstoragefile.c | 10 +++++----- tests/virstoragetest.c | 31 +++++++++++++++++++++++++------ tests/virstringtest.c | 3 +++ tests/viruritest.c | 1 + 4 files changed, 34 insertions(+), 11 deletions(-)
ACK series
Pushed; Thanks. Peter
participants (2)
-
Eric Blake -
Peter Krempa