All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] virtio-blk: limit seg_max to a safe value
@ 2021-05-18 15:04 Stefan Hajnoczi
  2021-05-19  7:57 ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2021-05-18 15:04 UTC (permalink / raw)
  To: linux-block
  Cc: jasowang, Michael S. Tsirkin, Christoph Hellwig, Xie Yongji,
	Stefan Hajnoczi

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(-)

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) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio-blk: limit seg_max to a safe value
  2021-05-18 15:04 [PATCH] virtio-blk: limit seg_max to a safe value Stefan Hajnoczi
@ 2021-05-19  7:57 ` Christoph Hellwig
  2021-05-19  8:40   ` Stefan Hajnoczi
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2021-05-19  7:57 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: linux-block, jasowang, Michael S. Tsirkin, Christoph Hellwig, Xie Yongji

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio-blk: limit seg_max to a safe value
  2021-05-19  7:57 ` Christoph Hellwig
@ 2021-05-19  8:40   ` Stefan Hajnoczi
  0 siblings, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2021-05-19  8:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-block, jasowang, Michael S. Tsirkin, Xie Yongji

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-19  8:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18 15:04 [PATCH] virtio-blk: limit seg_max to a safe value Stefan Hajnoczi
2021-05-19  7:57 ` Christoph Hellwig
2021-05-19  8:40   ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.