| Message ID | 20200603082141.42683-1-wipawel@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XTF] xenbus: fix xenbus_write() ring overflow | expand |
On 03/06/2020 09:21, Pawel Wieczorkiewicz wrote: > Currently the xenbus_write() does not handle ring wrapping around > correctly. When ring buffer is almost full and there is not enough > space for next packet (e.g. there is 12 bytes of space left, but the > packet header needs to transmit 16 bytes) the memcpy() goes out of the > ring buffer boundry. > Instead, the part variable should be limited to the space available in > the ring buffer, so the memcpy() can fill up the buffer, update len > variable (to indicate that there is still some data to be copied) and > thereby the xenbus_write() loop can iterate again to finish copying > the remainder of data to the beginning of the ring buffer. > > Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> Oops. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> and pushed.
diff --git a/common/xenbus.c b/common/xenbus.c index 59159f2..24fff48 100644 --- a/common/xenbus.c +++ b/common/xenbus.c @@ -31,9 +31,7 @@ static void xenbus_write(const void *data, size_t len) uint32_t prod = ACCESS_ONCE(xb_ring->req_prod); uint32_t cons = ACCESS_ONCE(xb_ring->req_cons); - uint32_t used = mask_xenbus_idx(prod - cons); - - part = (XENBUS_RING_SIZE - 1) - used; + part = (XENBUS_RING_SIZE - 1) - mask_xenbus_idx(prod - cons); /* No space? Kick xenstored and wait for it to consume some data. */ if ( !part ) @@ -47,7 +45,7 @@ static void xenbus_write(const void *data, size_t len) } /* Don't overrun the ring. */ - part = min(part, XENBUS_RING_SIZE - used); + part = min(part, XENBUS_RING_SIZE - mask_xenbus_idx(prod)); /* Don't write more than necessary. */ part = min(part, (unsigned int)len);
Currently the xenbus_write() does not handle ring wrapping around correctly. When ring buffer is almost full and there is not enough space for next packet (e.g. there is 12 bytes of space left, but the packet header needs to transmit 16 bytes) the memcpy() goes out of the ring buffer boundry. Instead, the part variable should be limited to the space available in the ring buffer, so the memcpy() can fill up the buffer, update len variable (to indicate that there is still some data to be copied) and thereby the xenbus_write() loop can iterate again to finish copying the remainder of data to the beginning of the ring buffer. Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> --- common/xenbus.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)