On Wed, Oct 04, 2023 at 04:55:02PM -0500, Eric Blake wrote:
> > > +static int
> > > +nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request,
> > > + Error **errp)
> >
> > [..]
> > > + for (i = 0; i < count; i++) {
> > > + id = ldl_be_p(buf + sizeof(NBDBlockStatusPayload) + sizeof(id) *
> > > i);
> > > + if (id == NBD_META_ID_BASE_ALLOCATION) {
> > > + if (request->contexts->base_allocation) {
> > > + goto skip;
> > > + }
> >
> > should we also check that base_allocation is negotiated?
>
> Oh, good point. Without that check, the client can pass in random id
> numbers that it never negotiated. I've queued 1-11 and will probably
> send a pull request for those this week, while respinning this patch
> to fix the remaining issues you pointed out.
I'm squashing in the following. If you can review it today, I'll
include it in my pull request this afternoon; if not, we still have
time before soft freeze to get it in the next batch.
diff --git i/nbd/server.c w/nbd/server.c
index 30816b42386..62654579cbc 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -2478,19 +2478,22 @@ nbd_co_block_status_payload_read(NBDClient *client,
NBDRequest *request,
for (i = 0; i < count; i++) {
id = ldl_be_p(buf + sizeof(NBDBlockStatusPayload) + sizeof(id) * i);
if (id == NBD_META_ID_BASE_ALLOCATION) {
- if (request->contexts->base_allocation) {
+ if (!client->contexts.base_allocation ||
+ request->contexts->base_allocation) {
goto skip;
}
request->contexts->base_allocation = true;
} else if (id == NBD_META_ID_ALLOCATION_DEPTH) {
- if (request->contexts->allocation_depth) {
+ if (!client->contexts.allocation_depth ||
+ request->contexts->allocation_depth) {
goto skip;
}
request->contexts->allocation_depth = true;
} else {
- int idx = id - NBD_META_ID_DIRTY_BITMAP;
+ unsigned idx = id - NBD_META_ID_DIRTY_BITMAP;
- if (idx > nr_bitmaps || request->contexts->bitmaps[idx]) {
+ if (idx > nr_bitmaps || !client->contexts.bitmaps[idx] ||
+ request->contexts->bitmaps[idx]) {
goto skip;
}
request->contexts->bitmaps[idx] = true;
diff --git i/nbd/trace-events w/nbd/trace-events
index 3cf2d00e458..00ae3216a11 100644
--- i/nbd/trace-events
+++ w/nbd/trace-events
@@ -70,7 +70,7 @@ nbd_co_send_chunk_read(uint64_t cookie, uint64_t offset, void
*data, uint64_t si
nbd_co_send_chunk_read_hole(uint64_t cookie, uint64_t offset, uint64_t size)
"Send structured read hole reply: cookie = %" PRIu64 ", offset = %" PRIu64 ",
len = %" PRIu64
nbd_co_send_extents(uint64_t cookie, unsigned int extents, uint32_t id,
uint64_t length, int last) "Send block status reply: cookie = %" PRIu64 ",
extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
nbd_co_send_chunk_error(uint64_t cookie, int err, const char *errname, const
char *msg) "Send structured error reply: cookie = %" PRIu64 ", error = %d (%s),
msg = '%s'"
-nbd_co_receive_block_status_payload_compliance(uint64_t from, int len) "client
sent unusable block status payload: from=0x%" PRIx64 ", len=0x%x"
+nbd_co_receive_block_status_payload_compliance(uint64_t from, uint64_t len)
"client sent unusable block status payload: from=0x%" PRIx64 ", len=0x%" PRIx64
nbd_co_receive_request_decode_type(uint64_t cookie, uint16_t type, const char
*name) "Decoding type: cookie = %" PRIu64 ", type = %" PRIu16 " (%s)"
nbd_co_receive_request_payload_received(uint64_t cookie, uint64_t len)
"Payload received: cookie = %" PRIu64 ", len = %" PRIu64
nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client
sent non-compliant write without payload flag: from=0x%" PRIx64 ", len=0x%"
PRIx64
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
