qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Connor Kuehl <ckuehl@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, dillaman@redhat.com, qemu-devel@nongnu.org,
	mreitz@redhat.com
Subject: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()
Date: Thu,  1 Apr 2021 10:52:11 -0500	[thread overview]
Message-ID: <20210401155211.2093139-3-ckuehl@redhat.com> (raw)
In-Reply-To: <20210401155211.2093139-1-ckuehl@redhat.com>

That's qemu_rbd_unescape()'s job! No need to duplicate the labor.

Furthermore, this was causing some confusion in the parsing logic to
where the caller might test for the presence of a character to split on
like so:

if (strchr(image_name, '/')) {
        found_str = qemu_rbd_next_tok(image_name, '/', &image_name);
	[..]

When qemu_rbd_next_tok() performs unescaping as a side effect, the
parser can get confused thinking that it can split this string, but
really the delimiter '/' gets unescaped and so qemu_rbd_next_tok() never
"finds" the delimiter and consumes the rest of the token input stream.

This is problematic because qemu_rbd_next_tok() also steals the input
pointer to the token stream and sets it to NULL. This causes a segfault
where the parser expects to split one string into two.

In this case, the parser is determining if a string is a
namespace/image_name pair like so:

	"foo/bar"

And clearly it's looking to split it like so:

	namespace:  foo
	image_name: bar

but if the input is "foo\/bar", it *should* split into

	namespace:  foo\
	image_name: bar

and its subordinate parts can be unescaped after tokenization.

So, instead of tokenizing *and* escaping all at once, do one before the
other to avoid stumbling into a segfault by confusing the parsing logic.

Reported-by: Han Han <hhan@redhat.com>
Fixes: https://bugzilla.redhat.com/1873913
Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
---
 block/rbd.c                | 3 ---
 tests/qemu-iotests/231     | 4 ++++
 tests/qemu-iotests/231.out | 3 +++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9071a00e3f..9bed0863e5 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -123,9 +123,6 @@ static char *qemu_rbd_next_tok(char *src, char delim, char **p)
         if (*end == delim) {
             break;
         }
-        if (*end == '\\' && end[1] != '\0') {
-            end++;
-        }
     }
     if (*end == delim) {
         *p = end + 1;
diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
index 0f66d0ca36..8e6c6447c1 100755
--- a/tests/qemu-iotests/231
+++ b/tests/qemu-iotests/231
@@ -55,6 +55,10 @@ _filter_conf()
 $QEMU_IMG info "json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 2>&1 | _filter_conf
 $QEMU_IMG info "json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}" 2>&1 | _filter_conf
 
+# Regression test: the qemu-img invocation is expected to fail, but it should
+# not seg fault the parser.
+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index 747dd221bb..a785a6e859 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory
+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
+unable to get monitor info from DNS SRV with service name: ceph-mon
+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or directory
 *** done
-- 
2.30.2



  parent reply	other threads:[~2021-04-01 15:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01 15:52 [PATCH 0/2] Fix segfault in qemu_rbd_parse_filename Connor Kuehl
2021-04-01 15:52 ` [PATCH 1/2] iotests/231: Update expected deprecation message Connor Kuehl
2021-04-01 16:52   ` Max Reitz
2021-04-01 17:07     ` Max Reitz
2021-04-01 17:10       ` Connor Kuehl
2021-04-01 15:52 ` Connor Kuehl [this message]
2021-04-01 17:24   ` [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok() Max Reitz
2021-04-01 18:09     ` Connor Kuehl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210401155211.2093139-3-ckuehl@redhat.com \
    --to=ckuehl@redhat.com \
    --cc=dillaman@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).