[PATCH RFC v2 22/25] kmsan: unpoisoning buffers from devices etc.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When data is copied to memory from a device KMSAN should treat it as
initialized. In most cases it's enough to just unpoison the buffer that
is known to come from a device.
In the case with __do_page_cache_readahead() and bio_copy_user_iov() we
have to mark the whole pages as ignored by KMSAN, as it's not obvious
where these pages are read again.

Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx>
To: Alexander Potapenko <glider@xxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Jens Axboe <axboe@xxxxxxxxx>
Cc: "Theodore Ts'o" <tytso@xxxxxxx>
Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Cc: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Eric Van Hensbergen <ericvh@xxxxxxxxx>
Cc: Takashi Iwai <tiwai@xxxxxxxx>
Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Cc: linux-mm@xxxxxxxxx

---
Suggestions on how to avoid scattering too many annotations over the
code are welcome. Perhaps annotating DMA reads/writes is enough to get
rid of most of these annotations.

v2:
 - dropped a call to kmsan_unpoison_shadow() in do_read_cache_page()

Change-Id: Id460e7a86ce564f1357469f53d0c7410ca08f0e9
---
 block/bio.c                  | 20 ++++++++++++++++++++
 block/partition-generic.c    |  9 ++++++++-
 drivers/char/random.c        |  2 ++
 drivers/input/serio/libps2.c |  6 +++++-
 drivers/scsi/scsi_lib.c      |  4 ++++
 drivers/usb/core/message.c   |  6 +++++-
 drivers/virtio/virtio_ring.c | 14 ++++++++++++++
 fs/buffer.c                  |  7 ++++++-
 include/linux/dma-mapping.h  |  2 ++
 include/linux/skbuff.h       |  5 ++++-
 mm/filemap.c                 |  1 +
 mm/readahead.c               |  6 ++++++
 net/9p/protocol.c            |  2 ++
 sound/core/oss/pcm_oss.c     |  7 +++++++
 14 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 8f0ed6228fc5..40773cf4b50c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/kmsan-checks.h>
 #include <linux/export.h>
 #include <linux/mempool.h>
 #include <linux/workqueue.h>
@@ -900,6 +901,13 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 			__bio_add_page(bio, page, len, offset);
 		}
 		offset = 0;
+		/*
+		 * TODO(glider): these pages will be soon passed to block
+		 * device for reading, so consider them initialized.
+		 */
+		if (iov_iter_rw(iter) == READ)
+			kmsan_unpoison_shadow(page_address(page),
+						PAGE_SIZE);
 	}
 
 	iov_iter_advance(iter, size);
@@ -1274,11 +1282,16 @@ struct bio *bio_copy_user_iov(struct request_queue *q,
 
 			i++;
 		} else {
+			/*
+			 * TODO(glider): KMSAN doesn't track the pages
+			 * allocated for bio here.
+			 */
 			page = alloc_page(q->bounce_gfp | gfp_mask);
 			if (!page) {
 				ret = -ENOMEM;
 				break;
 			}
+			kmsan_ignore_page(page, /*order*/0);
 		}
 
 		if (bio_add_pc_page(q, bio, page, bytes, offset) < bytes) {
@@ -1574,6 +1587,13 @@ struct bio *bio_copy_kern(struct request_queue *q, void *data, unsigned int len,
 		if (!page)
 			goto cleanup;
 
+		/*
+		 * TODO(glider): if we're about to read data from a SCSI device,
+		 * assume the page allocated for that is already initialized.
+		 */
+		if (reading)
+			kmsan_unpoison_shadow(page_address(page), PAGE_SIZE);
+
 		if (!reading)
 			memcpy(page_address(page), p, bytes);
 
diff --git a/block/partition-generic.c b/block/partition-generic.c
index aee643ce13d1..87b3ae9e7727 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -663,13 +663,20 @@ unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
 {
 	struct address_space *mapping = bdev->bd_inode->i_mapping;
 	struct page *page;
+	unsigned char *retval;
 
 	page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_SHIFT-9)), NULL);
 	if (!IS_ERR(page)) {
 		if (PageError(page))
 			goto fail;
 		p->v = page;
-		return (unsigned char *)page_address(page) +  ((n & ((1 << (PAGE_SHIFT - 9)) - 1)) << 9);
+		retval = (unsigned char *)page_address(page) +
+			 ((n & ((1 << (PAGE_SHIFT - 9)) - 1)) << 9);
+		/*
+		 * Unpoison sector-sized chunk of memory coming from the device.
+		 */
+		kmsan_unpoison_shadow(retval, SECTOR_SIZE);
+		return retval;
 fail:
 		put_page(page);
 	}
diff --git a/drivers/char/random.c b/drivers/char/random.c
index de434feb873a..cc4afc0f1039 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -320,6 +320,7 @@
 #include <linux/fs.h>
 #include <linux/genhd.h>
 #include <linux/interrupt.h>
+#include <linux/kmsan-checks.h>
 #include <linux/mm.h>
 #include <linux/nodemask.h>
 #include <linux/spinlock.h>
@@ -1061,6 +1062,7 @@ static void _extract_crng(struct crng_state *crng,
 	spin_lock_irqsave(&crng->lock, flags);
 	if (arch_get_random_long(&v))
 		crng->state[14] ^= v;
+	kmsan_unpoison_shadow(crng->state, sizeof(crng->state));
 	chacha20_block(&crng->state[0], out);
 	if (crng->state[12] == 0)
 		crng->state[13]++;
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index a8c94a940a79..80a9e0a9d3c3 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -8,6 +8,7 @@
 
 
 #include <linux/delay.h>
+#include <linux/kmsan-checks.h>
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/interrupt.h>
@@ -30,6 +31,7 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
 	int error;
 
 	lockdep_assert_held(&ps2dev->serio->lock);
+	kmsan_check_memory(&byte, 1);
 
 	do {
 		ps2dev->nak = 1;
@@ -294,9 +296,11 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
 
 	serio_pause_rx(ps2dev->serio);
 
-	if (param)
+	if (param) {
 		for (i = 0; i < receive; i++)
 			param[i] = ps2dev->cmdbuf[(receive - 1) - i];
+		kmsan_unpoison_shadow(param, receive);
+	}
 
 	if (ps2dev->cmdcnt &&
 	    (command != PS2_CMD_RESET_BAT || ps2dev->cmdcnt != 1)) {
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5447738906ac..a6d6a0eed8ec 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -14,6 +14,7 @@
 #include <linux/blkdev.h>
 #include <linux/completion.h>
 #include <linux/kernel.h>
+#include <linux/kmsan-checks.h>
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/pci.h>
@@ -296,6 +297,9 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	ret = rq->result;
  out:
 	blk_put_request(req);
+	/* TODO(glider): this is a bit rough. */
+	if (data_direction == DMA_FROM_DEVICE)
+		kmsan_unpoison_shadow(buffer, bufflen);
 
 	return ret;
 }
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 5adf489428aa..f1c3f8b3cfae 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -9,6 +9,7 @@
 #include <linux/usb.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/kmsan-checks.h>
 #include <linux/mm.h>
 #include <linux/timer.h>
 #include <linux/ctype.h>
@@ -101,8 +102,11 @@ static int usb_internal_control_msg(struct usb_device *usb_dev,
 	retv = usb_start_wait_urb(urb, timeout, &length);
 	if (retv < 0)
 		return retv;
-	else
+	else {
+		/* TODO(glider): USB initializes |length| bytes? */
+		kmsan_unpoison_shadow(data, length);
 		return length;
+	}
 }
 
 /**
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a8041e451e9e..19d35744e3e1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/hrtimer.h>
 #include <linux/dma-mapping.h>
+#include <linux/kmsan-checks.h>
 #include <xen/xen.h>
 
 #ifdef DEBUG
@@ -387,6 +388,12 @@ static void vring_unmap_one_split(const struct vring_virtqueue *vq,
 			       (flags & VRING_DESC_F_WRITE) ?
 			       DMA_FROM_DEVICE : DMA_TO_DEVICE);
 	}
+	/*
+	 * Unmapping DMA memory after a transfer from device requires this
+	 * memory to be unpoisoned.
+	 */
+	if (flags & VRING_DESC_F_WRITE)
+		kmsan_unpoison_shadow((const void *)desc->addr, desc->len);
 }
 
 static struct vring_desc *alloc_indirect_split(struct virtqueue *_vq,
@@ -500,6 +507,13 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
 			desc[i].flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_NEXT | VRING_DESC_F_WRITE);
 			desc[i].addr = cpu_to_virtio64(_vq->vdev, addr);
 			desc[i].len = cpu_to_virtio32(_vq->vdev, sg->length);
+			/*
+			 * It's hard to figure out the buffer's address upon
+			 * receive. Instead we unpoison it once, when exposing
+			 * it to the device, and hope nobody else will write to
+			 * it.
+			 */
+			kmsan_unpoison_shadow(sg_virt(sg), sg->length);
 			prev = i;
 			i = virtio16_to_cpu(_vq->vdev, desc[i].next);
 		}
diff --git a/fs/buffer.c b/fs/buffer.c
index 86a38b979323..c08fb1a77fa1 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1170,14 +1170,17 @@ static struct buffer_head *__bread_slow(struct buffer_head *bh)
 	lock_buffer(bh);
 	if (buffer_uptodate(bh)) {
 		unlock_buffer(bh);
+		kmsan_unpoison_shadow(bh->b_data, bh->b_size);
 		return bh;
 	} else {
 		get_bh(bh);
 		bh->b_end_io = end_buffer_read_sync;
 		submit_bh(REQ_OP_READ, 0, bh);
 		wait_on_buffer(bh);
-		if (buffer_uptodate(bh))
+		if (buffer_uptodate(bh)) {
+			kmsan_unpoison_shadow(bh->b_data, bh->b_size);
 			return bh;
+		}
 	}
 	brelse(bh);
 	return NULL;
@@ -1320,6 +1323,8 @@ __getblk_gfp(struct block_device *bdev, sector_t block,
 	might_sleep();
 	if (bh == NULL)
 		bh = __getblk_slow(bdev, block, size, gfp);
+	if (bh)
+		kmsan_unpoison_shadow(bh->b_data, bh->b_size);
 	return bh;
 }
 EXPORT_SYMBOL(__getblk_gfp);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fca475a..cc7b05eb024f 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/dma-debug.h>
 #include <linux/dma-direction.h>
+#include <linux/kmsan-checks.h>
 #include <linux/scatterlist.h>
 #include <linux/bug.h>
 #include <linux/mem_encrypt.h>
@@ -281,6 +282,7 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 	dma_addr_t addr;
 
+	kmsan_unpoison_shadow(page_address(page), size);
 	BUG_ON(!valid_dma_direction(dir));
 	if (dma_is_direct(ops))
 		addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7914fdaf4226..1efe136250c9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -11,6 +11,7 @@
 #define _LINUX_SKBUFF_H
 
 #include <linux/kernel.h>
+#include <linux/kmsan-checks.h>
 #include <linux/compiler.h>
 #include <linux/time.h>
 #include <linux/bug.h>
@@ -2214,6 +2215,8 @@ static inline void *skb_put_data(struct sk_buff *skb, const void *data,
 {
 	void *tmp = skb_put(skb, len);
 
+	/* Unpoison the data received from the network device. */
+	kmsan_unpoison_shadow(data, len);
 	memcpy(tmp, data, len);
 
 	return tmp;
@@ -2221,7 +2224,7 @@ static inline void *skb_put_data(struct sk_buff *skb, const void *data,
 
 static inline void skb_put_u8(struct sk_buff *skb, u8 val)
 {
-	*(u8 *)skb_put(skb, 1) = val;
+	*(u8 *)skb_put(skb, 1) = KMSAN_INIT_VALUE(val);
 }
 
 void *skb_push(struct sk_buff *skb, unsigned int len);
diff --git a/mm/filemap.c b/mm/filemap.c
index 85b7d087eb45..05b930e9f484 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -18,6 +18,7 @@
 #include <linux/uaccess.h>
 #include <linux/capability.h>
 #include <linux/kernel_stat.h>
+#include <linux/kmsan-checks.h>
 #include <linux/gfp.h>
 #include <linux/mm.h>
 #include <linux/swap.h>
diff --git a/mm/readahead.c b/mm/readahead.c
index 2fe72cd29b47..1fe61128095b 100644
--- a/mm/readahead.c
+++ b/mm/readahead.c
@@ -193,7 +193,13 @@ unsigned int __do_page_cache_readahead(struct address_space *mapping,
 			continue;
 		}
 
+		/*
+		 * The easiest way to handle these pages is to mark them
+		 * untracked by KMSAN, assuming they're never used by anything
+		 * else.
+		 */
 		page = __page_cache_alloc(gfp_mask);
+		kmsan_ignore_page(page, /*order*/0);
 		if (!page)
 			break;
 		page->index = page_offset;
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index 03593eb240d8..5d88ed181422 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
+#include <linux/kmsan-checks.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
@@ -46,6 +47,7 @@ EXPORT_SYMBOL(p9stat_free);
 size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size)
 {
 	size_t len = min(pdu->size - pdu->offset, size);
+	kmsan_unpoison_shadow(&pdu->sdata[pdu->offset], len);
 	memcpy(data, &pdu->sdata[pdu->offset], len);
 	pdu->offset += len;
 	return size - len;
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index f57c610d7523..c3795e495196 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -12,6 +12,7 @@
 #endif
 
 #include <linux/init.h>
+#include <linux/kmsan-checks.h>	/* for kmsan_unpoison_shadow() */
 #include <linux/slab.h>
 #include <linux/sched/signal.h>
 #include <linux/time.h>
@@ -1054,6 +1055,12 @@ static int snd_pcm_oss_change_params_locked(struct snd_pcm_substream *substream)
 		err = -ENOMEM;
 		goto failure;
 	}
+	/*
+	 * Unpoison the freshly created buffer to prevent KMSAN from reporting
+	 * uninit errors.
+	 * TODO(glider): unpoison it only when it's actually initialized.
+	 */
+	kmsan_unpoison_shadow(runtime->oss.buffer, runtime->oss.period_bytes);
 
 	runtime->oss.params = 0;
 	runtime->oss.prepare = 1;
-- 
2.24.0.rc0.303.g954a862665-goog





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux