Skip to content

Commit

Permalink
nbd/client: Use smarter assert
Browse files Browse the repository at this point in the history
Assigning strlen() to a uint32_t and then asserting that it isn't too
large doesn't catch the case of an input string 4G in length.
Thankfully, the incoming strings can never be that large: if the
export name or query is reflecting a string the client got from the
server, we already guarantee that we dropped the NBD connection if the
server sent more than 32M in a single reply to our NBD_OPT_* request;
if the export name is coming from qemu, nbd_receive_negotiate()
asserted that strlen(info->name) <= NBD_MAX_STRING_SIZE; and
similarly, a query string via x->dirty_bitmap coming from the user was
bounds-checked in either qemu-nbd or by the limitations of QMP.
Still, it doesn't hurt to be more explicit in how we write our
assertions to not have to analyze whether inadvertent wraparound is
possible.

Fixes: 93676c8 ("nbd: Don't send oversize strings", v4.2.0)
Reported-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230608135653.2918540-2-eblake@redhat.com>
  • Loading branch information
ebblake committed Jul 19, 2023
1 parent 414c0cf commit f47b6ea
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions nbd/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -650,19 +650,20 @@ static int nbd_send_meta_query(QIOChannel *ioc, uint32_t opt,
Error **errp)
{
int ret;
uint32_t export_len = strlen(export);
uint32_t export_len;
uint32_t queries = !!query;
uint32_t query_len = 0;
uint32_t data_len;
char *data;
char *p;

assert(strnlen(export, NBD_MAX_STRING_SIZE + 1) <= NBD_MAX_STRING_SIZE);
export_len = strlen(export);
data_len = sizeof(export_len) + export_len + sizeof(queries);
assert(export_len <= NBD_MAX_STRING_SIZE);
if (query) {
assert(strnlen(query, NBD_MAX_STRING_SIZE + 1) <= NBD_MAX_STRING_SIZE);
query_len = strlen(query);
data_len += sizeof(query_len) + query_len;
assert(query_len <= NBD_MAX_STRING_SIZE);
} else {
assert(opt == NBD_OPT_LIST_META_CONTEXT);
}
Expand Down

0 comments on commit f47b6ea

Please sign in to comment.