Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

xen/usb: don't use gnttab_end_foreign_access() in xenhcd_gnttab_done()

The usage of gnttab_end_foreign_access() in xenhcd_gnttab_done() is
not safe against a malicious backend, as the backend could keep the
I/O page mapped and modify it even after the granted memory page is
being used for completely other purposes in the local system.

So replace that use case with gnttab_try_end_foreign_access() and
disable the PV host adapter in case the backend didn't stop using the
granted page.

In xenhcd_urb_request_done() immediately return in case of setting
the device state to "error" instead of looking into further backend
responses.

Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V2:
- use gnttab_try_end_foreign_access()

+18 -8
+18 -8
drivers/usb/host/xen-hcd.c
··· 716 716 return 0; 717 717 } 718 718 719 - static void xenhcd_gnttab_done(struct usb_shadow *shadow) 719 + static void xenhcd_gnttab_done(struct xenhcd_info *info, unsigned int id) 720 720 { 721 + struct usb_shadow *shadow = info->shadow + id; 721 722 int nr_segs = 0; 722 723 int i; 723 724 ··· 727 726 if (xenusb_pipeisoc(shadow->req.pipe)) 728 727 nr_segs += shadow->req.u.isoc.nr_frame_desc_segs; 729 728 730 - for (i = 0; i < nr_segs; i++) 731 - gnttab_end_foreign_access(shadow->req.seg[i].gref, 0, 0UL); 729 + for (i = 0; i < nr_segs; i++) { 730 + if (!gnttab_try_end_foreign_access(shadow->req.seg[i].gref)) 731 + xenhcd_set_error(info, "backend didn't release grant"); 732 + } 732 733 733 734 shadow->req.nr_buffer_segs = 0; 734 735 shadow->req.u.isoc.nr_frame_desc_segs = 0; ··· 844 841 list_for_each_entry_safe(urbp, tmp, &info->in_progress_list, list) { 845 842 req_id = urbp->req_id; 846 843 if (!urbp->unlinked) { 847 - xenhcd_gnttab_done(&info->shadow[req_id]); 844 + xenhcd_gnttab_done(info, req_id); 845 + if (info->error) 846 + return; 848 847 if (urbp->urb->status == -EINPROGRESS) 849 848 /* not dequeued */ 850 849 xenhcd_giveback_urb(info, urbp->urb, ··· 947 942 rp = info->urb_ring.sring->rsp_prod; 948 943 if (RING_RESPONSE_PROD_OVERFLOW(&info->urb_ring, rp)) { 949 944 xenhcd_set_error(info, "Illegal index on urb-ring"); 950 - spin_unlock_irqrestore(&info->lock, flags); 951 - return 0; 945 + goto err; 952 946 } 953 947 rmb(); /* ensure we see queued responses up to "rp" */ 954 948 ··· 956 952 id = res.id; 957 953 if (id >= XENUSB_URB_RING_SIZE) { 958 954 xenhcd_set_error(info, "Illegal data on urb-ring"); 959 - continue; 955 + goto err; 960 956 } 961 957 962 958 if (likely(xenusb_pipesubmit(info->shadow[id].req.pipe))) { 963 - xenhcd_gnttab_done(&info->shadow[id]); 959 + xenhcd_gnttab_done(info, id); 960 + if (info->error) 961 + goto err; 964 962 urb = info->shadow[id].urb; 965 963 if (likely(urb)) { 966 964 urb->actual_length = res.actual_length; ··· 984 978 spin_unlock_irqrestore(&info->lock, flags); 985 979 986 980 return more_to_do; 981 + 982 + err: 983 + spin_unlock_irqrestore(&info->lock, flags); 984 + return 0; 987 985 } 988 986 989 987 static int xenhcd_conn_notify(struct xenhcd_info *info)