Skip to content

Commit 6d1c2f4

Browse files
jgross1jgross1
committed
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 <jgross@suse.com> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Link: https://lore.kernel.org/r/20220428075323.12853-5-jgross@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
1 parent a2f6751 commit 6d1c2f4

1 file changed

Lines changed: 76 additions & 28 deletions

File tree

‎drivers/scsi/xen-scsifront.c‎

Lines changed: 76 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ struct vscsifrnt_shadow {
8383
uint16_t rqid;
8484
uint16_t ref_rqid;
8585

86+
bool inflight;
87+
8688
unsigned int nr_grants; /* number of grants in gref[] */
8789
struct scsiif_request_segment *sg; /* scatter/gather elements */
8890
struct scsiif_request_segment seg[VSCSIIF_SG_TABLESIZE];
@@ -104,7 +106,11 @@ struct vscsifrnt_info {
104106
struct xenbus_device *dev;
105107

106108
struct Scsi_Host *host;
107-
int host_active;
109+
enum {
110+
STATE_INACTIVE,
111+
STATE_ACTIVE,
112+
STATE_ERROR
113+
} host_active;
108114

109115
unsigned int evtchn;
110116
unsigned int irq;
@@ -217,13 +223,22 @@ static int scsifront_do_request(struct vscsifrnt_info *info,
217223
for (i = 0; i < (shadow->nr_segments & ~VSCSIIF_SG_GRANT); i++)
218224
ring_req->seg[i] = shadow->seg[i];
219225

226+
shadow->inflight = true;
227+
220228
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify);
221229
if (notify)
222230
notify_remote_via_irq(info->irq);
223231

224232
return 0;
225233
}
226234

235+
static void scsifront_set_error(struct vscsifrnt_info *info, const char *msg)
236+
{
237+
shost_printk(KERN_ERR, info->host, KBUILD_MODNAME "%s\n"
238+
"Disabling device for further use\n", msg);
239+
info->host_active = STATE_ERROR;
240+
}
241+
227242
static void scsifront_gnttab_done(struct vscsifrnt_info *info,
228243
struct vscsifrnt_shadow *shadow)
229244
{
@@ -234,9 +249,8 @@ static void scsifront_gnttab_done(struct vscsifrnt_info *info,
234249

235250
for (i = 0; i < shadow->nr_grants; i++) {
236251
if (unlikely(!gnttab_try_end_foreign_access(shadow->gref[i]))) {
237-
shost_printk(KERN_ALERT, info->host, KBUILD_MODNAME
238-
"grant still in use by backend\n");
239-
BUG();
252+
scsifront_set_error(info, "grant still in use by backend");
253+
return;
240254
}
241255
}
242256

@@ -308,6 +322,8 @@ static void scsifront_cdb_cmd_done(struct vscsifrnt_info *info,
308322
BUG_ON(sc == NULL);
309323

310324
scsifront_gnttab_done(info, shadow);
325+
if (info->host_active == STATE_ERROR)
326+
return;
311327
scsifront_put_rqid(info, id);
312328

313329
set_host_byte(sc, scsifront_host_byte(ring_rsp->rslt));
@@ -348,9 +364,7 @@ static void scsifront_sync_cmd_done(struct vscsifrnt_info *info,
348364
scsifront_wake_up(info);
349365
return;
350366
default:
351-
shost_printk(KERN_ERR, info->host, KBUILD_MODNAME
352-
"bad reset state %d, possibly leaking %u\n",
353-
shadow->rslt_reset, id);
367+
scsifront_set_error(info, "bad reset state");
354368
break;
355369
}
356370
spin_unlock_irqrestore(&info->shadow_lock, flags);
@@ -361,28 +375,41 @@ static void scsifront_sync_cmd_done(struct vscsifrnt_info *info,
361375
static void scsifront_do_response(struct vscsifrnt_info *info,
362376
struct vscsiif_response *ring_rsp)
363377
{
364-
if (WARN(ring_rsp->rqid >= VSCSIIF_MAX_REQS ||
365-
test_bit(ring_rsp->rqid, info->shadow_free_bitmap),
366-
"illegal rqid %u returned by backend!\n", ring_rsp->rqid))
378+
struct vscsifrnt_shadow *shadow;
379+
380+
if (ring_rsp->rqid >= VSCSIIF_MAX_REQS ||
381+
!info->shadow[ring_rsp->rqid]->inflight) {
382+
scsifront_set_error(info, "illegal rqid returned by backend!");
367383
return;
384+
}
385+
shadow = info->shadow[ring_rsp->rqid];
386+
shadow->inflight = false;
368387

369-
if (info->shadow[ring_rsp->rqid]->act == VSCSIIF_ACT_SCSI_CDB)
388+
if (shadow->act == VSCSIIF_ACT_SCSI_CDB)
370389
scsifront_cdb_cmd_done(info, ring_rsp);
371390
else
372391
scsifront_sync_cmd_done(info, ring_rsp);
373392
}
374393

375-
static int scsifront_ring_drain(struct vscsifrnt_info *info)
394+
static int scsifront_ring_drain(struct vscsifrnt_info *info,
395+
unsigned int *eoiflag)
376396
{
377-
struct vscsiif_response *ring_rsp;
397+
struct vscsiif_response ring_rsp;
378398
RING_IDX i, rp;
379399
int more_to_do = 0;
380400

381-
rp = info->ring.sring->rsp_prod;
382-
rmb(); /* ordering required respective to dom0 */
401+
rp = READ_ONCE(info->ring.sring->rsp_prod);
402+
virt_rmb(); /* ordering required respective to backend */
403+
if (RING_RESPONSE_PROD_OVERFLOW(&info->ring, rp)) {
404+
scsifront_set_error(info, "illegal number of responses");
405+
return 0;
406+
}
383407
for (i = info->ring.rsp_cons; i != rp; i++) {
384-
ring_rsp = RING_GET_RESPONSE(&info->ring, i);
385-
scsifront_do_response(info, ring_rsp);
408+
RING_COPY_RESPONSE(&info->ring, i, &ring_rsp);
409+
scsifront_do_response(info, &ring_rsp);
410+
if (info->host_active == STATE_ERROR)
411+
return 0;
412+
*eoiflag &= ~XEN_EOI_FLAG_SPURIOUS;
386413
}
387414

388415
info->ring.rsp_cons = i;
@@ -395,14 +422,15 @@ static int scsifront_ring_drain(struct vscsifrnt_info *info)
395422
return more_to_do;
396423
}
397424

398-
static int scsifront_cmd_done(struct vscsifrnt_info *info)
425+
static int scsifront_cmd_done(struct vscsifrnt_info *info,
426+
unsigned int *eoiflag)
399427
{
400428
int more_to_do;
401429
unsigned long flags;
402430

403431
spin_lock_irqsave(info->host->host_lock, flags);
404432

405-
more_to_do = scsifront_ring_drain(info);
433+
more_to_do = scsifront_ring_drain(info, eoiflag);
406434

407435
info->wait_ring_available = 0;
408436

@@ -416,20 +444,28 @@ static int scsifront_cmd_done(struct vscsifrnt_info *info)
416444
static irqreturn_t scsifront_irq_fn(int irq, void *dev_id)
417445
{
418446
struct vscsifrnt_info *info = dev_id;
447+
unsigned int eoiflag = XEN_EOI_FLAG_SPURIOUS;
448+
449+
if (info->host_active == STATE_ERROR) {
450+
xen_irq_lateeoi(irq, XEN_EOI_FLAG_SPURIOUS);
451+
return IRQ_HANDLED;
452+
}
419453

420-
while (scsifront_cmd_done(info))
454+
while (scsifront_cmd_done(info, &eoiflag))
421455
/* Yield point for this unbounded loop. */
422456
cond_resched();
423457

458+
xen_irq_lateeoi(irq, eoiflag);
459+
424460
return IRQ_HANDLED;
425461
}
426462

427463
static void scsifront_finish_all(struct vscsifrnt_info *info)
428464
{
429-
unsigned i;
465+
unsigned int i, dummy;
430466
struct vscsiif_response resp;
431467

432-
scsifront_ring_drain(info);
468+
scsifront_ring_drain(info, &dummy);
433469

434470
for (i = 0; i < VSCSIIF_MAX_REQS; i++) {
435471
if (test_bit(i, info->shadow_free_bitmap))
@@ -586,6 +622,9 @@ static int scsifront_queuecommand(struct Scsi_Host *shost,
586622
unsigned long flags;
587623
int err;
588624

625+
if (info->host_active == STATE_ERROR)
626+
return SCSI_MLQUEUE_HOST_BUSY;
627+
589628
sc->result = 0;
590629

591630
shadow->sc = sc;
@@ -638,6 +677,9 @@ static int scsifront_action_handler(struct scsi_cmnd *sc, uint8_t act)
638677
struct vscsifrnt_shadow *shadow, *s = scsi_cmd_priv(sc);
639678
int err = 0;
640679

680+
if (info->host_active == STATE_ERROR)
681+
return FAILED;
682+
641683
shadow = kzalloc(sizeof(*shadow), GFP_NOIO);
642684
if (!shadow)
643685
return FAILED;
@@ -709,6 +751,9 @@ static int scsifront_sdev_configure(struct scsi_device *sdev)
709751
struct vscsifrnt_info *info = shost_priv(sdev->host);
710752
int err;
711753

754+
if (info->host_active == STATE_ERROR)
755+
return -EIO;
756+
712757
if (info && current == info->curr) {
713758
err = xenbus_printf(XBT_NIL, info->dev->nodename,
714759
info->dev_state_path, "%d", XenbusStateConnected);
@@ -784,7 +829,7 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info)
784829
goto free_gnttab;
785830
}
786831

787-
err = bind_evtchn_to_irq(info->evtchn);
832+
err = bind_evtchn_to_irq_lateeoi(info->evtchn);
788833
if (err <= 0) {
789834
xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq");
790835
goto free_gnttab;
@@ -914,7 +959,7 @@ static int scsifront_probe(struct xenbus_device *dev,
914959
goto free_sring;
915960
}
916961
info->host = host;
917-
info->host_active = 1;
962+
info->host_active = STATE_ACTIVE;
918963

919964
xenbus_switch_state(dev, XenbusStateInitialised);
920965

@@ -982,10 +1027,10 @@ static int scsifront_remove(struct xenbus_device *dev)
9821027
pr_debug("%s: %s removed\n", __func__, dev->nodename);
9831028

9841029
mutex_lock(&scsifront_mutex);
985-
if (info->host_active) {
1030+
if (info->host_active != STATE_INACTIVE) {
9861031
/* Scsi_host not yet removed */
9871032
scsi_remove_host(info->host);
988-
info->host_active = 0;
1033+
info->host_active = STATE_INACTIVE;
9891034
}
9901035
mutex_unlock(&scsifront_mutex);
9911036

@@ -1009,9 +1054,9 @@ static void scsifront_disconnect(struct vscsifrnt_info *info)
10091054
*/
10101055

10111056
mutex_lock(&scsifront_mutex);
1012-
if (info->host_active) {
1057+
if (info->host_active != STATE_INACTIVE) {
10131058
scsi_remove_host(host);
1014-
info->host_active = 0;
1059+
info->host_active = STATE_INACTIVE;
10151060
}
10161061
mutex_unlock(&scsifront_mutex);
10171062

@@ -1029,6 +1074,9 @@ static void scsifront_do_lun_hotplug(struct vscsifrnt_info *info, int op)
10291074
unsigned int hst, chn, tgt, lun;
10301075
struct scsi_device *sdev;
10311076

1077+
if (info->host_active == STATE_ERROR)
1078+
return;
1079+
10321080
dir = xenbus_directory(XBT_NIL, dev->otherend, "vscsi-devs", &dir_n);
10331081
if (IS_ERR(dir))
10341082
return;

0 commit comments

Comments
 (0)