From a2f6751d5af44d8c1cab93c2c410fd334c460339 Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 28 Apr 2022 09:53:22 +0200 Subject: xen/scsifront: use new command result macros Add a translation layer for the command result values. Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Link: https://lore.kernel.org/r/20220428075323.12853-4-jgross@suse.com Signed-off-by: Juergen Gross --- drivers/scsi/xen-scsifront.c | 64 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 8 deletions(-) (limited to 'drivers/scsi') diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 12109e4c73d4..8511bfc62963 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -243,6 +243,56 @@ static void scsifront_gnttab_done(struct vscsifrnt_info *info, kfree(shadow->sg); } +static unsigned int scsifront_host_byte(int32_t rslt) +{ + switch (XEN_VSCSIIF_RSLT_HOST(rslt)) { + case XEN_VSCSIIF_RSLT_HOST_OK: + return DID_OK; + case XEN_VSCSIIF_RSLT_HOST_NO_CONNECT: + return DID_NO_CONNECT; + case XEN_VSCSIIF_RSLT_HOST_BUS_BUSY: + return DID_BUS_BUSY; + case XEN_VSCSIIF_RSLT_HOST_TIME_OUT: + return DID_TIME_OUT; + case XEN_VSCSIIF_RSLT_HOST_BAD_TARGET: + return DID_BAD_TARGET; + case XEN_VSCSIIF_RSLT_HOST_ABORT: + return DID_ABORT; + case XEN_VSCSIIF_RSLT_HOST_PARITY: + return DID_PARITY; + case XEN_VSCSIIF_RSLT_HOST_ERROR: + return DID_ERROR; + case XEN_VSCSIIF_RSLT_HOST_RESET: + return DID_RESET; + case XEN_VSCSIIF_RSLT_HOST_BAD_INTR: + return DID_BAD_INTR; + case XEN_VSCSIIF_RSLT_HOST_PASSTHROUGH: + return DID_PASSTHROUGH; + case XEN_VSCSIIF_RSLT_HOST_SOFT_ERROR: + return DID_SOFT_ERROR; + case XEN_VSCSIIF_RSLT_HOST_IMM_RETRY: + return DID_IMM_RETRY; + case XEN_VSCSIIF_RSLT_HOST_REQUEUE: + return DID_REQUEUE; + case XEN_VSCSIIF_RSLT_HOST_TRANSPORT_DISRUPTED: + return DID_TRANSPORT_DISRUPTED; + case XEN_VSCSIIF_RSLT_HOST_TRANSPORT_FAILFAST: + return DID_TRANSPORT_FAILFAST; + case XEN_VSCSIIF_RSLT_HOST_TARGET_FAILURE: + return DID_TARGET_FAILURE; + case XEN_VSCSIIF_RSLT_HOST_NEXUS_FAILURE: + return DID_NEXUS_FAILURE; + case XEN_VSCSIIF_RSLT_HOST_ALLOC_FAILURE: + return DID_ALLOC_FAILURE; + case XEN_VSCSIIF_RSLT_HOST_MEDIUM_ERROR: + return DID_MEDIUM_ERROR; + case XEN_VSCSIIF_RSLT_HOST_TRANSPORT_MARGINAL: + return DID_TRANSPORT_MARGINAL; + default: + return DID_ERROR; + } +} + static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info, struct vscsiif_response *ring_rsp) { @@ -250,7 +300,6 @@ static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info, struct scsi_cmnd *sc; uint32_t id; uint8_t sense_len; - int result; id = ring_rsp->rqid; shadow = info->shadow[id]; @@ -261,12 +310,8 @@ static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info, scsifront_gnttab_done(info, shadow); scsifront_put_rqid(info, id); - result = ring_rsp->rslt; - if (result >> 24) - set_host_byte(sc, DID_ERROR); - else - set_host_byte(sc, host_byte(result)); - set_status_byte(sc, result & 0xff); + set_host_byte(sc, scsifront_host_byte(ring_rsp->rslt)); + set_status_byte(sc, XEN_VSCSIIF_RSLT_STATUS(ring_rsp->rslt)); scsi_set_resid(sc, ring_rsp->residual_len); sense_len = min_t(uint8_t, VSCSIIF_SENSE_BUFFERSIZE, @@ -290,7 +335,10 @@ static void scsifront_sync_cmd_done(struct vscsifrnt_info *info, shadow->wait_reset = 1; switch (shadow->rslt_reset) { case RSLT_RESET_WAITING: - shadow->rslt_reset = ring_rsp->rslt; + if (ring_rsp->rslt == XEN_VSCSIIF_RSLT_RESET_SUCCESS) + shadow->rslt_reset = SUCCESS; + else + shadow->rslt_reset = FAILED; break; case RSLT_RESET_ERR: kick = _scsifront_put_rqid(info, id); -- cgit v1.2.3-70-g09d2 From 6d1c2f48f3fcf5f42967938c8ba6b42ac6b44b43 Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 28 Apr 2022 09:53:23 +0200 Subject: xen/scsifront: harden driver against malicious backend Instead of relying on a well behaved PV scsi backend verify all meta data received from the backend and avoid multiple reads of the same data from the shared ring page. In case any illegal data from the backend is detected switch the PV device to a new "error" state and deactivate it for further use. Use the "lateeoi" variant for the event channel in order to avoid event storms blocking the guest. Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Link: https://lore.kernel.org/r/20220428075323.12853-5-jgross@suse.com Signed-off-by: Juergen Gross --- drivers/scsi/xen-scsifront.c | 104 +++++++++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 28 deletions(-) (limited to 'drivers/scsi') diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 8511bfc62963..56173beecbc6 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -83,6 +83,8 @@ struct vscsifrnt_shadow { uint16_t rqid; uint16_t ref_rqid; + bool inflight; + unsigned int nr_grants; /* number of grants in gref[] */ struct scsiif_request_segment *sg; /* scatter/gather elements */ struct scsiif_request_segment seg[VSCSIIF_SG_TABLESIZE]; @@ -104,7 +106,11 @@ struct vscsifrnt_info { struct xenbus_device *dev; struct Scsi_Host *host; - int host_active; + enum { + STATE_INACTIVE, + STATE_ACTIVE, + STATE_ERROR + } host_active; unsigned int evtchn; unsigned int irq; @@ -217,6 +223,8 @@ static int scsifront_do_request(struct vscsifrnt_info *info, for (i = 0; i < (shadow->nr_segments & ~VSCSIIF_SG_GRANT); i++) ring_req->seg[i] = shadow->seg[i]; + shadow->inflight = true; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify); if (notify) notify_remote_via_irq(info->irq); @@ -224,6 +232,13 @@ static int scsifront_do_request(struct vscsifrnt_info *info, return 0; } +static void scsifront_set_error(struct vscsifrnt_info *info, const char *msg) +{ + shost_printk(KERN_ERR, info->host, KBUILD_MODNAME "%s\n" + "Disabling device for further use\n", msg); + info->host_active = STATE_ERROR; +} + static void scsifront_gnttab_done(struct vscsifrnt_info *info, struct vscsifrnt_shadow *shadow) { @@ -234,9 +249,8 @@ static void scsifront_gnttab_done(struct vscsifrnt_info *info, for (i = 0; i < shadow->nr_grants; i++) { if (unlikely(!gnttab_try_end_foreign_access(shadow->gref[i]))) { - shost_printk(KERN_ALERT, info->host, KBUILD_MODNAME - "grant still in use by backend\n"); - BUG(); + scsifront_set_error(info, "grant still in use by backend"); + return; } } @@ -308,6 +322,8 @@ static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info, BUG_ON(sc == NULL); scsifront_gnttab_done(info, shadow); + if (info->host_active == STATE_ERROR) + return; scsifront_put_rqid(info, id); set_host_byte(sc, scsifront_host_byte(ring_rsp->rslt)); @@ -348,9 +364,7 @@ static void scsifront_sync_cmd_done(struct vscsifrnt_info *info, scsifront_wake_up(info); return; default: - shost_printk(KERN_ERR, info->host, KBUILD_MODNAME - "bad reset state %d, possibly leaking %u\n", - shadow->rslt_reset, id); + scsifront_set_error(info, "bad reset state"); break; } spin_unlock_irqrestore(&info->shadow_lock, flags); @@ -361,28 +375,41 @@ static void scsifront_sync_cmd_done(struct vscsifrnt_info *info, static void scsifront_do_response(struct vscsifrnt_info *info, struct vscsiif_response *ring_rsp) { - if (WARN(ring_rsp->rqid >= VSCSIIF_MAX_REQS || - test_bit(ring_rsp->rqid, info->shadow_free_bitmap), - "illegal rqid %u returned by backend!\n", ring_rsp->rqid)) + struct vscsifrnt_shadow *shadow; + + if (ring_rsp->rqid >= VSCSIIF_MAX_REQS || + !info->shadow[ring_rsp->rqid]->inflight) { + scsifront_set_error(info, "illegal rqid returned by backend!"); return; + } + shadow = info->shadow[ring_rsp->rqid]; + shadow->inflight = false; - if (info->shadow[ring_rsp->rqid]->act == VSCSIIF_ACT_SCSI_CDB) + if (shadow->act == VSCSIIF_ACT_SCSI_CDB) scsifront_cdb_cmd_done(info, ring_rsp); else scsifront_sync_cmd_done(info, ring_rsp); } -static int scsifront_ring_drain(struct vscsifrnt_info *info) +static int scsifront_ring_drain(struct vscsifrnt_info *info, + unsigned int *eoiflag) { - struct vscsiif_response *ring_rsp; + struct vscsiif_response ring_rsp; RING_IDX i, rp; int more_to_do = 0; - rp = info->ring.sring->rsp_prod; - rmb(); /* ordering required respective to dom0 */ + rp = READ_ONCE(info->ring.sring->rsp_prod); + virt_rmb(); /* ordering required respective to backend */ + if (RING_RESPONSE_PROD_OVERFLOW(&info->ring, rp)) { + scsifront_set_error(info, "illegal number of responses"); + return 0; + } for (i = info->ring.rsp_cons; i != rp; i++) { - ring_rsp = RING_GET_RESPONSE(&info->ring, i); - scsifront_do_response(info, ring_rsp); + RING_COPY_RESPONSE(&info->ring, i, &ring_rsp); + scsifront_do_response(info, &ring_rsp); + if (info->host_active == STATE_ERROR) + return 0; + *eoiflag &= ~XEN_EOI_FLAG_SPURIOUS; } info->ring.rsp_cons = i; @@ -395,14 +422,15 @@ static int scsifront_ring_drain(struct vscsifrnt_info *info) return more_to_do; } -static int scsifront_cmd_done(struct vscsifrnt_info *info) +static int scsifront_cmd_done(struct vscsifrnt_info *info, + unsigned int *eoiflag) { int more_to_do; unsigned long flags; spin_lock_irqsave(info->host->host_lock, flags); - more_to_do = scsifront_ring_drain(info); + more_to_do = scsifront_ring_drain(info, eoiflag); info->wait_ring_available = 0; @@ -416,20 +444,28 @@ static int scsifront_cmd_done(struct vscsifrnt_info *info) static irqreturn_t scsifront_irq_fn(int irq, void *dev_id) { struct vscsifrnt_info *info = dev_id; + unsigned int eoiflag = XEN_EOI_FLAG_SPURIOUS; + + if (info->host_active == STATE_ERROR) { + xen_irq_lateeoi(irq, XEN_EOI_FLAG_SPURIOUS); + return IRQ_HANDLED; + } - while (scsifront_cmd_done(info)) + while (scsifront_cmd_done(info, &eoiflag)) /* Yield point for this unbounded loop. */ cond_resched(); + xen_irq_lateeoi(irq, eoiflag); + return IRQ_HANDLED; } static void scsifront_finish_all(struct vscsifrnt_info *info) { - unsigned i; + unsigned int i, dummy; struct vscsiif_response resp; - scsifront_ring_drain(info); + scsifront_ring_drain(info, &dummy); for (i = 0; i < VSCSIIF_MAX_REQS; i++) { if (test_bit(i, info->shadow_free_bitmap)) @@ -586,6 +622,9 @@ static int scsifront_queuecommand(struct Scsi_Host *shost, unsigned long flags; int err; + if (info->host_active == STATE_ERROR) + return SCSI_MLQUEUE_HOST_BUSY; + sc->result = 0; shadow->sc = sc; @@ -638,6 +677,9 @@ static int scsifront_action_handler(struct scsi_cmnd *sc, uint8_t act) struct vscsifrnt_shadow *shadow, *s = scsi_cmd_priv(sc); int err = 0; + if (info->host_active == STATE_ERROR) + return FAILED; + shadow = kzalloc(sizeof(*shadow), GFP_NOIO); if (!shadow) return FAILED; @@ -709,6 +751,9 @@ static int scsifront_sdev_configure(struct scsi_device *sdev) struct vscsifrnt_info *info = shost_priv(sdev->host); int err; + if (info->host_active == STATE_ERROR) + return -EIO; + if (info && current == info->curr) { err = xenbus_printf(XBT_NIL, info->dev->nodename, info->dev_state_path, "%d", XenbusStateConnected); @@ -784,7 +829,7 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info) goto free_gnttab; } - err = bind_evtchn_to_irq(info->evtchn); + err = bind_evtchn_to_irq_lateeoi(info->evtchn); if (err <= 0) { xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq"); goto free_gnttab; @@ -914,7 +959,7 @@ static int scsifront_probe(struct xenbus_device *dev, goto free_sring; } info->host = host; - info->host_active = 1; + info->host_active = STATE_ACTIVE; xenbus_switch_state(dev, XenbusStateInitialised); @@ -982,10 +1027,10 @@ static int scsifront_remove(struct xenbus_device *dev) pr_debug("%s: %s removed\n", __func__, dev->nodename); mutex_lock(&scsifront_mutex); - if (info->host_active) { + if (info->host_active != STATE_INACTIVE) { /* Scsi_host not yet removed */ scsi_remove_host(info->host); - info->host_active = 0; + info->host_active = STATE_INACTIVE; } mutex_unlock(&scsifront_mutex); @@ -1009,9 +1054,9 @@ static void scsifront_disconnect(struct vscsifrnt_info *info) */ mutex_lock(&scsifront_mutex); - if (info->host_active) { + if (info->host_active != STATE_INACTIVE) { scsi_remove_host(host); - info->host_active = 0; + info->host_active = STATE_INACTIVE; } mutex_unlock(&scsifront_mutex); @@ -1029,6 +1074,9 @@ static void scsifront_do_lun_hotplug(struct vscsifrnt_info *info, int op) unsigned int hst, chn, tgt, lun; struct scsi_device *sdev; + if (info->host_active == STATE_ERROR) + return; + dir = xenbus_directory(XBT_NIL, dev->otherend, "vscsi-devs", &dir_n); if (IS_ERR(dir)) return; -- cgit v1.2.3-70-g09d2 From 70920be6ff0d7abe92311a678cb6c84fcec658ca Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 28 Apr 2022 09:01:02 +0200 Subject: xen/scsifront: remove unused GRANT_INVALID_REF definition GRANT_INVALID_REF isn't used in scsifront, so remove it. Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/scsi/xen-scsifront.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/scsi') diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 56173beecbc6..4c55e479fc36 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -58,9 +58,6 @@ #include - -#define GRANT_INVALID_REF 0 - #define VSCSIFRONT_OP_ADD_LUN 1 #define VSCSIFRONT_OP_DEL_LUN 2 #define VSCSIFRONT_OP_READD_LUN 3 -- cgit v1.2.3-70-g09d2 From caa427d252031e2f97228a474ff320fdc54171af Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 28 Apr 2022 09:01:04 +0200 Subject: xen/scsifront: use xenbus_setup_ring() and xenbus_teardown_ring() Simplify scsifront's ring creation and removal via xenbus_setup_ring() and xenbus_teardown_ring(). Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/scsi/xen-scsifront.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) (limited to 'drivers/scsi') diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 4c55e479fc36..51afc66e839d 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -798,27 +798,15 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info) { struct xenbus_device *dev = info->dev; struct vscsiif_sring *sring; - grant_ref_t gref; - int err = -ENOMEM; + int err; /***** Frontend to Backend ring start *****/ - sring = (struct vscsiif_sring *)__get_free_page(GFP_KERNEL); - if (!sring) { - xenbus_dev_fatal(dev, err, - "fail to allocate shared ring (Front to Back)"); + err = xenbus_setup_ring(dev, GFP_KERNEL, (void **)&sring, 1, + &info->ring_ref); + if (err) return err; - } - SHARED_RING_INIT(sring); - FRONT_RING_INIT(&info->ring, sring, PAGE_SIZE); - err = xenbus_grant_ring(dev, sring, 1, &gref); - if (err < 0) { - free_page((unsigned long)sring); - xenbus_dev_fatal(dev, err, - "fail to grant shared ring (Front to Back)"); - return err; - } - info->ring_ref = gref; + XEN_FRONT_RING_INIT(&info->ring, sring, PAGE_SIZE); err = xenbus_alloc_evtchn(dev, &info->evtchn); if (err) { @@ -847,8 +835,7 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info) free_irq: unbind_from_irqhandler(info->irq, info); free_gnttab: - gnttab_end_foreign_access(info->ring_ref, - (unsigned long)info->ring.sring); + xenbus_teardown_ring((void **)&sring, 1, &info->ring_ref); return err; } @@ -856,8 +843,7 @@ free_gnttab: static void scsifront_free_ring(struct vscsifrnt_info *info) { unbind_from_irqhandler(info->irq, info); - gnttab_end_foreign_access(info->ring_ref, - (unsigned long)info->ring.sring); + xenbus_teardown_ring((void **)&info->ring.sring, 1, &info->ring_ref); } static int scsifront_init_ring(struct vscsifrnt_info *info) -- cgit v1.2.3-70-g09d2