| Message ID | alpine.DEB.2.02.1601061201020.31111@kaball.uk.xensource.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/01/16 12:08, Stefano Stabellini wrote: > If the frontend sets out_cons to a value higher than out_prod, it will > cause xenfb_handle_events to loop about 2^32 times. Avoid that by using > better checks at the beginning of the function. You can't use less than to compare prod and cons because they wrap. You need to compare (prod - cons) against ring size (or similar) to check for overflow. See RING_REQUEST_PROD_OVERFLOW() etc. David
> -----Original Message----- > From: qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org > [mailto:qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org] On > Behalf Of Stefano Stabellini > Sent: 06 January 2016 12:08 > To: qemu-devel@nongnu.org > Cc: liuling-it@360.cn; xen-devel@lists.xensource.com; Stefano Stabellini > Subject: [Qemu-devel] [PATCH] xenfb.c: avoid expensive loops when prod > <= out_cons > > If the frontend sets out_cons to a value higher than out_prod, it will > cause xenfb_handle_events to loop about 2^32 times. Avoid that by using > better checks at the beginning of the function. > What happens when out_prod wraps? Paul > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 4e2a27a..f963cf2 100644 > --- a/hw/display/xenfb.c > +++ b/hw/display/xenfb.c > @@ -789,10 +789,11 @@ static void xenfb_handle_events(struct XenFB > *xenfb) > > prod = page->out_prod; > out_cons = page->out_cons; > - if (prod == out_cons) > - return; > + if (prod <= out_cons) { > + return; > + } > xen_rmb(); /* ensure we see ring contents up to prod */ > - for (cons = out_cons; cons != prod; cons++) { > + for (cons = out_cons; cons < prod; cons++) { > union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, > cons); > uint8_t type = event->type; > int x, y, w, h;
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 4e2a27a..f963cf2 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -789,10 +789,11 @@ static void xenfb_handle_events(struct XenFB *xenfb) prod = page->out_prod; out_cons = page->out_cons; - if (prod == out_cons) - return; + if (prod <= out_cons) { + return; + } xen_rmb(); /* ensure we see ring contents up to prod */ - for (cons = out_cons; cons != prod; cons++) { + for (cons = out_cons; cons < prod; cons++) { union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons); uint8_t type = event->type; int x, y, w, h;
If the frontend sets out_cons to a value higher than out_prod, it will cause xenfb_handle_events to loop about 2^32 times. Avoid that by using better checks at the beginning of the function. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>