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
