Skip to content
Permalink
Browse files

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 <jgarber1@ualberta.ca>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
  • Loading branch information...
Jacob Garber authored and avpatel committed Aug 9, 2019
1 parent 6ddf71e commit 5e4021a2f5ca346d1c12b80d346c1a2e7eb4b501
Showing with 4 additions and 2 deletions.
  1. +4 −2 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)

0 comments on commit 5e4021a

Please sign in to comment.
You can’t perform that action at this time.