| Message ID | 20170804151440.320927-12-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > We set s->reply.handle to 0 on one error path and don't set on another. > For consistancy and to avoid assert in nbd_read_reply_entry let's > set s->reply.handle to 0 in case of wrong handle too. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/nbd-client.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Can this assertion be triggered now (presumably, with a broken server)? I'm trying to figure out if this is 2.10 material. [Urgh. If a broken server is able to cause an assertion failure that causes a client to abort on an assertion failure, that probably deserves a CVE]
07.08.2017 14:55, Eric Blake wrote: > On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: >> We set s->reply.handle to 0 on one error path and don't set on another. >> For consistancy and to avoid assert in nbd_read_reply_entry let's >> set s->reply.handle to 0 in case of wrong handle too. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/nbd-client.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) > Can this assertion be triggered now (presumably, with a broken server)? > I'm trying to figure out if this is 2.10 material. > > [Urgh. If a broken server is able to cause an assertion failure that > causes a client to abort on an assertion failure, that probably deserves > a CVE] > Hmm looks like I've mistaken, if handle is wrong than read_reply_co should be already finished, so it's impossible
diff --git a/block/nbd-client.c b/block/nbd-client.c index d6965d24db..b84cab4079 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -183,13 +183,13 @@ static int nbd_co_request(BlockDriverState *bs, reply.error = EIO; } } - - /* Tell the read handler to read another header. */ - s->reply.handle = 0; } rc = -reply.error; out: + /* Tell the read handler to read another header. */ + s->reply.handle = 0; + s->recv_coroutine[i] = NULL; /* Kick the read_reply_co to get the next reply. */
We set s->reply.handle to 0 on one error path and don't set on another. For consistancy and to avoid assert in nbd_read_reply_entry let's set s->reply.handle to 0 in case of wrong handle too. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/nbd-client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)