| Message ID | 20210518150415.152730-1-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-blk: limit seg_max to a safe value | expand |
On Tue, May 18, 2021 at 04:04:15PM +0100, Stefan Hajnoczi wrote: > + /* Prevent integer overflows and excessive blk-mq req cmd_size */ > + sg_elems = min_t(u32, sg_elems, SG_MAX_SEGMENTS); Please pick your own constant here instead of abusing some kernel implementation detail (that is fairly SCSI specific to start with). It might be useful to also document such limits, even if just advisory, in the virtio spec.
On Wed, May 19, 2021 at 09:57:02AM +0200, Christoph Hellwig wrote: > On Tue, May 18, 2021 at 04:04:15PM +0100, Stefan Hajnoczi wrote: > > + /* Prevent integer overflows and excessive blk-mq req cmd_size */ > > + sg_elems = min_t(u32, sg_elems, SG_MAX_SEGMENTS); > > Please pick your own constant here instead of abusing some kernel > implementation detail (that is fairly SCSI specific to start with). > > It might be useful to also document such limits, even if just advisory, > in the virtio spec. Thanks, will fix in v2. Stefan
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index b9fa3ef5b57c..4dfd5fc7aeea 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -728,7 +728,10 @@ static int virtblk_probe(struct virtio_device *vdev) if (err || !sg_elems) sg_elems = 1; - /* We need an extra sg elements at head and tail. */ + /* Prevent integer overflows and excessive blk-mq req cmd_size */ + sg_elems = min_t(u32, sg_elems, SG_MAX_SEGMENTS); + + /* We need extra sg elements at head and tail. */ sg_elems += 2; vdev->priv = vblk = kmalloc(sizeof(*vblk), GFP_KERNEL); if (!vblk) {
The struct virtio_blk_config seg_max value is read from the device and incremented by 2 to account for the request header and status byte descriptors added by the driver. In preparation for supporting untrusted virtio-blk devices, protect against integer overflow and limit the value to a safe maximum (SG_MAX_SEGMENTS). Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- drivers/block/virtio_blk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)