From 5e4021a2f5ca346d1c12b80d346c1a2e7eb4b501 Mon Sep 17 00:00:00 2001 From: Jacob Garber Date: Fri, 9 Aug 2019 17:13:59 -0600 Subject: [PATCH] lib: Prevent unintended sign extensions In the last argument to sbi_memset() we essentially have the following multiplication: size_t = u16 * u16 Due to C's integer semantics, both u16's are implicitly converted to int before the multiplication, which cannot hold all possible values of a u16 * u16. If the multiplication overflows, the intermediate result will be a negative number. On 64-bit platforms, this will be sign-extended to a huge integer in the conversion to a u64 (aka size_t). Being the size argument to sbi_memset(), this could potentially cause a large out-of-bounds write. The solution is to manually cast one of the u16 to a size_t, which will make it large enough to avoid the implicit conversion and any overflow. Signed-off-by: Jacob Garber Reviewed-by: Bin Meng Reviewed-by: Atish Patra --- lib/sbi/sbi_fifo.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/sbi/sbi_fifo.c b/lib/sbi/sbi_fifo.c index e69009f..18ff0d6 100644 --- a/lib/sbi/sbi_fifo.c +++ b/lib/sbi/sbi_fifo.c @@ -20,7 +20,7 @@ void sbi_fifo_init(struct sbi_fifo *fifo, void *queue_mem, u16 entries, fifo->entry_size = entry_size; SPIN_LOCK_INIT(&fifo->qlock); fifo->avail = fifo->tail = 0; - sbi_memset(fifo->queue, 0, entries * entry_size); + sbi_memset(fifo->queue, 0, (size_t)entries * entry_size); } /* Note: must be called with fifo->qlock held */ @@ -74,9 +74,11 @@ bool sbi_fifo_is_empty(struct sbi_fifo *fifo) /* Note: must be called with fifo->qlock held */ static inline void __sbi_fifo_reset(struct sbi_fifo *fifo) { + size_t size = (size_t)fifo->num_entries * fifo->entry_size; + fifo->avail = 0; fifo->tail = 0; - sbi_memset(fifo->queue, 0, fifo->num_entries * fifo->entry_size); + sbi_memset(fifo->queue, 0, size); } bool sbi_fifo_reset(struct sbi_fifo *fifo) -- 2.7.4