9:26 a.m.
On 05.10.23 16:49, Eric Blake wrote:
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] ||
Oops, I didn't notice: s/>/>=/, as nr_bitmaps is length of array.
+ 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
--
Best regards,
Vladimir