Skip to content

Commit aff477c

Browse files
jgross1gregkh
authored andcommitted
xen/usb: harden xen_hcd against malicious backends
Make sure a malicious backend can't cause any harm other than wrong I/O data. Missing are verification of the request id in a response, sanitizing the reported actual I/O length, and protection against interrupt storms from the backend. Signed-off-by: Juergen Gross <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7f220d4 commit aff477c

File tree

1 file changed

+43
-14
lines changed

1 file changed

+43
-14
lines changed

drivers/usb/host/xen-hcd.c

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ struct vdevice_status {
5151
struct usb_shadow {
5252
struct xenusb_urb_request req;
5353
struct urb *urb;
54+
bool in_flight;
5455
};
5556

5657
struct xenhcd_info {
@@ -720,6 +721,12 @@ static void xenhcd_gnttab_done(struct xenhcd_info *info, unsigned int id)
720721
int nr_segs = 0;
721722
int i;
722723

724+
if (!shadow->in_flight) {
725+
xenhcd_set_error(info, "Illegal request id");
726+
return;
727+
}
728+
shadow->in_flight = false;
729+
723730
nr_segs = shadow->req.nr_buffer_segs;
724731

725732
if (xenusb_pipeisoc(shadow->req.pipe))
@@ -803,6 +810,7 @@ static int xenhcd_do_request(struct xenhcd_info *info, struct urb_priv *urbp)
803810

804811
info->urb_ring.req_prod_pvt++;
805812
info->shadow[id].urb = urb;
813+
info->shadow[id].in_flight = true;
806814

807815
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&info->urb_ring, notify);
808816
if (notify)
@@ -931,10 +939,27 @@ static int xenhcd_unlink_urb(struct xenhcd_info *info, struct urb_priv *urbp)
931939
return ret;
932940
}
933941

934-
static int xenhcd_urb_request_done(struct xenhcd_info *info)
942+
static void xenhcd_res_to_urb(struct xenhcd_info *info,
943+
struct xenusb_urb_response *res, struct urb *urb)
944+
{
945+
if (unlikely(!urb))
946+
return;
947+
948+
if (res->actual_length > urb->transfer_buffer_length)
949+
urb->actual_length = urb->transfer_buffer_length;
950+
else if (res->actual_length < 0)
951+
urb->actual_length = 0;
952+
else
953+
urb->actual_length = res->actual_length;
954+
urb->error_count = res->error_count;
955+
urb->start_frame = res->start_frame;
956+
xenhcd_giveback_urb(info, urb, res->status);
957+
}
958+
959+
static int xenhcd_urb_request_done(struct xenhcd_info *info,
960+
unsigned int *eoiflag)
935961
{
936962
struct xenusb_urb_response res;
937-
struct urb *urb;
938963
RING_IDX i, rp;
939964
__u16 id;
940965
int more_to_do = 0;
@@ -961,16 +986,12 @@ static int xenhcd_urb_request_done(struct xenhcd_info *info)
961986
xenhcd_gnttab_done(info, id);
962987
if (info->error)
963988
goto err;
964-
urb = info->shadow[id].urb;
965-
if (likely(urb)) {
966-
urb->actual_length = res.actual_length;
967-
urb->error_count = res.error_count;
968-
urb->start_frame = res.start_frame;
969-
xenhcd_giveback_urb(info, urb, res.status);
970-
}
989+
xenhcd_res_to_urb(info, &res, info->shadow[id].urb);
971990
}
972991

973992
xenhcd_add_id_to_freelist(info, id);
993+
994+
*eoiflag = 0;
974995
}
975996
info->urb_ring.rsp_cons = i;
976997

@@ -988,7 +1009,7 @@ static int xenhcd_urb_request_done(struct xenhcd_info *info)
9881009
return 0;
9891010
}
9901011

991-
static int xenhcd_conn_notify(struct xenhcd_info *info)
1012+
static int xenhcd_conn_notify(struct xenhcd_info *info, unsigned int *eoiflag)
9921013
{
9931014
struct xenusb_conn_response res;
9941015
struct xenusb_conn_request *req;
@@ -1033,6 +1054,8 @@ static int xenhcd_conn_notify(struct xenhcd_info *info)
10331054
info->conn_ring.req_prod_pvt);
10341055
req->id = id;
10351056
info->conn_ring.req_prod_pvt++;
1057+
1058+
*eoiflag = 0;
10361059
}
10371060

10381061
if (rc != info->conn_ring.req_prod_pvt)
@@ -1055,14 +1078,19 @@ static int xenhcd_conn_notify(struct xenhcd_info *info)
10551078
static irqreturn_t xenhcd_int(int irq, void *dev_id)
10561079
{
10571080
struct xenhcd_info *info = (struct xenhcd_info *)dev_id;
1081+
unsigned int eoiflag = XEN_EOI_FLAG_SPURIOUS;
10581082

1059-
if (unlikely(info->error))
1083+
if (unlikely(info->error)) {
1084+
xen_irq_lateeoi(irq, XEN_EOI_FLAG_SPURIOUS);
10601085
return IRQ_HANDLED;
1086+
}
10611087

1062-
while (xenhcd_urb_request_done(info) | xenhcd_conn_notify(info))
1088+
while (xenhcd_urb_request_done(info, &eoiflag) |
1089+
xenhcd_conn_notify(info, &eoiflag))
10631090
/* Yield point for this unbounded loop. */
10641091
cond_resched();
10651092

1093+
xen_irq_lateeoi(irq, eoiflag);
10661094
return IRQ_HANDLED;
10671095
}
10681096

@@ -1139,9 +1167,9 @@ static int xenhcd_setup_rings(struct xenbus_device *dev,
11391167
goto fail;
11401168
}
11411169

1142-
err = bind_evtchn_to_irq(info->evtchn);
1170+
err = bind_evtchn_to_irq_lateeoi(info->evtchn);
11431171
if (err <= 0) {
1144-
xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq");
1172+
xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq_lateeoi");
11451173
goto fail;
11461174
}
11471175

@@ -1494,6 +1522,7 @@ static struct usb_hcd *xenhcd_create_hcd(struct xenbus_device *dev)
14941522
for (i = 0; i < XENUSB_URB_RING_SIZE; i++) {
14951523
info->shadow[i].req.id = i + 1;
14961524
info->shadow[i].urb = NULL;
1525+
info->shadow[i].in_flight = false;
14971526
}
14981527
info->shadow[XENUSB_URB_RING_SIZE - 1].req.id = 0x0fff;
14991528

0 commit comments

Comments
 (0)