[PATCH] xen/scsifront: check for a NULL shadow entry on a backend response

[PATCH] xen/scsifront: check for a NULL shadow entry on a backend response

[Yehyeong Lee]

scsifront_do_response() validates the rqid in a backend response against
VSCSIIF_MAX_REQS before using it to index info->shadow[], but then
dereferences the entry to test ->inflight without checking that the slot
is populated.  Shadow slots are NULL before a command is submitted (the
host private area is zeroed at allocation) and are reset to NULL in
_scsifront_put_rqid() once a request completes, so an in-range rqid that
does not correspond to an outstanding request makes the frontend
dereference a NULL pointer.

A malicious or buggy backend can thus crash the guest by returning a
response whose rqid is in range but not in flight -- for example a
spurious response before any command has been issued, or a duplicate of
one already completed.  The ring has only VSCSIIF_MAX_REQS (16) slots, so
before the first command every in-range rqid selects a NULL slot.

Reject a response whose shadow slot is not populated.

Fixes: 6d1c2f48f3fc ("xen/scsifront: harden driver against malicious backend")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 drivers/scsi/xen-scsifront.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 989bcaee42caf..aa44a233eac8d 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -367,6 +367,7 @@ static void scsifront_do_response(struct vscsifrnt_info *info,
 	struct vscsifrnt_shadow *shadow;
 
 	if (ring_rsp->rqid >= VSCSIIF_MAX_REQS ||
+	    !info->shadow[ring_rsp->rqid] ||
 	    !info->shadow[ring_rsp->rqid]->inflight) {
 		scsifront_set_error(info, "illegal rqid returned by backend!");
 		return;
-- 
2.43.0

Re: [PATCH] xen/scsifront: check for a NULL shadow entry on a backend response

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1246 bytes --]

On 05.10.26 14:35, Yehyeong Lee wrote:
> scsifront_do_response() validates the rqid in a backend response against
> VSCSIIF_MAX_REQS before using it to index info->shadow[], but then
> dereferences the entry to test ->inflight without checking that the slot
> is populated.  Shadow slots are NULL before a command is submitted (the
> host private area is zeroed at allocation) and are reset to NULL in
> _scsifront_put_rqid() once a request completes, so an in-range rqid that
> does not correspond to an outstanding request makes the frontend
> dereference a NULL pointer.
> 
> A malicious or buggy backend can thus crash the guest by returning a
> response whose rqid is in range but not in flight -- for example a
> spurious response before any command has been issued, or a duplicate of
> one already completed.  The ring has only VSCSIIF_MAX_REQS (16) slots, so
> before the first command every in-range rqid selects a NULL slot.
> 
> Reject a response whose shadow slot is not populated.
> 
> Fixes: 6d1c2f48f3fc ("xen/scsifront: harden driver against malicious backend")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]