Skip to content

Commit a853b64

Browse files
committed
[KERNEL] Check if reply size matches request
Output according messages and abort connection if the reply didn't contain exactly as many bytes as requested, instead of going on and running into desync with the reply stream from the server, which would just give us confusing messages about header magic mismatch.
1 parent 22b178a commit a853b64

File tree

1 file changed

+32
-13
lines changed

1 file changed

+32
-13
lines changed

src/kernel/net.c

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ static void dnbd3_recv_workfn(struct work_struct *work)
467467
void *kaddr;
468468
unsigned long irqflags;
469469
uint16_t rid;
470-
int remaining;
470+
u32 remaining;
471471
int ret;
472472

473473
dnbd3_dev_dbg_cur(dev, "starting receive worker...\n");
@@ -528,16 +528,24 @@ static void dnbd3_recv_workfn(struct work_struct *work)
528528
goto out_unlock;
529529
}
530530
// receive data and answer to block layer
531+
remaining = reply_hdr.size;
531532
#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 14, 0)
532533
rq_for_each_segment(bvec_inst, blk_request, iter) {
533534
#else
534535
rq_for_each_segment(bvec, blk_request, iter) {
535536
#endif
536-
kaddr = kmap(bvec->bv_page) + bvec->bv_offset;
537-
iov.iov_base = kaddr;
538-
iov.iov_len = bvec->bv_len;
539-
ret = kernel_recvmsg(dev->sock, &msg, &iov, 1, bvec->bv_len, msg.msg_flags);
540-
kunmap(bvec->bv_page);
537+
if (bvec->bv_len > remaining) {
538+
dnbd3_dev_dbg_cur(
539+
dev, "request has more data remaining than is left in reply (want: %u, have: %u)\n",
540+
bvec->bv_len, remaining);
541+
ret = -1;
542+
} else {
543+
kaddr = kmap(bvec->bv_page) + bvec->bv_offset;
544+
iov.iov_base = kaddr;
545+
iov.iov_len = bvec->bv_len;
546+
ret = kernel_recvmsg(dev->sock, &msg, &iov, 1, bvec->bv_len, msg.msg_flags);
547+
kunmap(bvec->bv_page);
548+
}
541549
if (ret != bvec->bv_len) {
542550
if (ret == 0) {
543551
/* have not received any data, but remote peer is shutdown properly */
@@ -546,18 +554,29 @@ static void dnbd3_recv_workfn(struct work_struct *work)
546554
} else if (ret < 0) {
547555
if (!dnbd3_flag_taken(dev->connection_lock))
548556
dnbd3_dev_err_cur(dev,
549-
"disconnect: receiving from net to block layer\n");
557+
"receiving from net to block layer failed (ret=%d)\n", ret);
550558
} else {
551559
if (!dnbd3_flag_taken(dev->connection_lock))
552560
dnbd3_dev_err_cur(dev,
553561
"receiving from net to block layer (%d bytes)\n", ret);
554562
}
555-
// Requeue request
556-
spin_lock_irqsave(&dev->send_queue_lock, irqflags);
557-
list_add(&blk_request->queuelist, &dev->send_queue);
558-
spin_unlock_irqrestore(&dev->send_queue_lock, irqflags);
559-
goto out_unlock;
563+
goto segment_loop_end;
560564
}
565+
remaining -= ret;
566+
}
567+
segment_loop_end: /* Make this a goto as rq_for_each_segment is opaque and can be any number of nested loops */
568+
if (remaining != 0) {
569+
if (ret > 0) {
570+
/* No previous error, the reply must've had more payload than the according request */
571+
dnbd3_dev_err_cur(dev,
572+
"reply has payload left, but block request already satisfied (len: %u, remaining: %u)\n",
573+
reply_hdr.size, remaining);
574+
}
575+
// Requeue request
576+
spin_lock_irqsave(&dev->send_queue_lock, irqflags);
577+
list_add(&blk_request->queuelist, &dev->send_queue);
578+
spin_unlock_irqrestore(&dev->send_queue_lock, irqflags);
579+
goto out_unlock;
561580
}
562581
blk_mq_end_request(blk_request, BLK_STS_OK);
563582
break;
@@ -567,7 +586,7 @@ static void dnbd3_recv_workfn(struct work_struct *work)
567586
if (dev->use_server_provided_alts) {
568587
dnbd3_server_entry_t new_server;
569588

570-
while (remaining >= sizeof(dnbd3_server_entry_t)) {
589+
while (remaining >= sizeof(new_server)) {
571590
if (dnbd3_recv_bytes(dev->sock, &new_server, sizeof(new_server))
572591
!= sizeof(new_server)) {
573592
if (!dnbd3_flag_taken(dev->connection_lock))

0 commit comments

Comments
 (0)