2003-06-21 22:55:31

by Lou Langholtz

[permalink] [raw]
Subject: [PATCH] nbd driver 2.5+: fix for incorrect struct bio usage

diff -urN linux-2.5.72-p1/drivers/block/nbd.c linux-2.5.72-p2/drivers/block/nbd.c
--- linux-2.5.72-p1/drivers/block/nbd.c 2003-06-21 15:30:17.860967573 -0600
+++ linux-2.5.72-p2/drivers/block/nbd.c 2003-06-21 16:42:00.865099598 -0600
@@ -28,7 +28,10 @@
* the transmit lock. <[email protected]>
* 02-10-11 Allow hung xmit to be aborted via SIGKILL & various fixes.
* <[email protected]> <[email protected]>
- * 03-06-21 Fix memory corruption from module removal. <[email protected]>
+ * 03-06-21 Make nbd work with linux 2.5 block layer code. This fixes memory
+ * corruption from module removal too. <[email protected]>
+ * 03-06-21 Fix incorrect bio struct access that could have lead to memory
+ * corruption on receiving disk data. <[email protected]>
*
* possible FIXME: make set_sock / set_blksize / set_size / do_it one syscall
* why not: would need verify_area and friends, would share yet another
@@ -256,6 +259,12 @@
return NULL;
}

+static inline int sock_recv_bvec(struct socket *sock, struct bio_vec *bvec)
+{
+ return nbd_xmit(0, sock, page_address(bvec->bv_page) + bvec->bv_offset,
+ bvec->bv_len, MSG_WAITALL);
+}
+
#define HARDFAIL( s ) { printk( KERN_ERR "NBD: " s "(result %d)\n", result ); lo->harderror = result; return NULL; }
struct request *nbd_read_stat(struct nbd_device *lo)
/* NULL returned = something went wrong, inform userspace */
@@ -263,10 +272,11 @@
int result;
struct nbd_reply reply;
struct request *req;
+ struct socket *sock = lo->sock;

DEBUG("reading control, ");
reply.magic = 0;
- result = nbd_xmit(0, lo->sock, (char *) &reply, sizeof(reply), MSG_WAITALL);
+ result = nbd_xmit(0, sock, (char *) &reply, sizeof(reply), MSG_WAITALL);
if (result <= 0)
HARDFAIL("Recv control failed.");
req = nbd_find_request(lo, reply.handle);
@@ -280,14 +290,17 @@
FAIL("Other side returned error.");

if (nbd_cmd(req) == NBD_CMD_READ) {
- struct bio *bio = req->bio;
+ int i;
+ struct bio *bio;
DEBUG("data, ");
- do {
- result = nbd_xmit(0, lo->sock, bio_data(bio), bio->bi_size, MSG_WAITALL);
- if (result <= 0)
- HARDFAIL("Recv data failed.");
- bio = bio->bi_next;
- } while(bio);
+ rq_for_each_bio(bio, req) {
+ struct bio_vec *bvec;
+ bio_for_each_segment(bvec, bio, i) {
+ result = sock_recv_bvec(sock, bvec);
+ if (result <= 0)
+ HARDFAIL("Recv data failed.");
+ }
+ }
}
DEBUG("done.\n");
return req;


Attachments:
nbd-patch2 (2.39 kB)

2003-06-22 08:39:24

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] nbd driver 2.5+: fix for incorrect struct bio usage

On Sat, Jun 21 2003, Lou Langholtz wrote:
> Here's a possible patch #2... I believe the address pointed to by
> bio_data(bio) is not always contiguous over the length of bio->bi_size
> and was responsible for locking my machine up sometimes. My biggest
> reason for apprehension on believeing that I'm 100% correct on this is
> that there's still what appears to be a source of memory corruption in
> the patchlet modified nbd driver even after this patch. I know the
> driver is still not correctly notifying processes of the bytesize on
> open but the size reported appears to be big enough. Anyway, thanks for
> all of the feedback so far!!!!

You are correct, the current code breaks down for multi-page bio's. It
looks like you are missing a kmap still though, bvec->bv_page could be a
highmem page.

--
Jens Axboe