diff options
author | Michael Brown | 2016-02-11 19:44:24 +0100 |
---|---|---|
committer | Michael Brown | 2016-02-11 20:04:23 +0100 |
commit | 12b3b578869d5c25a32edd81950e104a286643d7 (patch) | |
tree | a6c9250db7e8986c130c91a67cab4f3d74258db2 /src/core | |
parent | [malloc] Guard against unsigned integer overflow (diff) | |
download | ipxe-12b3b578869d5c25a32edd81950e104a286643d7.tar.gz ipxe-12b3b578869d5c25a32edd81950e104a286643d7.tar.xz ipxe-12b3b578869d5c25a32edd81950e104a286643d7.zip |
[iobuf] Improve robustness of I/O buffer allocation
Guard against various corner cases (such as zero-length buffers, zero
alignments, and integer overflow when rounding up allocation lengths
and alignments) and ensure that the struct io_buffer is correctly
aligned even when the caller requests a non-zero alignment for the I/O
buffer itself.
Add self-tests to verify that the resulting alignments and lengths are
correct for a range of allocations.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/iobuf.c | 43 |
1 files changed, 34 insertions, 9 deletions
diff --git a/src/core/iobuf.c b/src/core/iobuf.c index 3e52ada4..0ee53e03 100644 --- a/src/core/iobuf.c +++ b/src/core/iobuf.c @@ -47,20 +47,45 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); */ struct io_buffer * alloc_iob_raw ( size_t len, size_t align, size_t offset ) { struct io_buffer *iobuf; + size_t padding; + size_t threshold; + unsigned int align_log2; void *data; - /* Align buffer length to ensure that struct io_buffer is aligned */ - len = ( len + __alignof__ ( *iobuf ) - 1 ) & - ~( __alignof__ ( *iobuf ) - 1 ); + /* Calculate padding required below alignment boundary to + * ensure that a correctly aligned inline struct io_buffer + * could fit (regardless of the requested offset). + */ + padding = ( sizeof ( *iobuf ) + __alignof__ ( *iobuf ) - 1 ); - /* Round up alignment to the nearest power of two */ - align = ( 1 << fls ( align - 1 ) ); + /* Round up requested alignment to at least the size of the + * padding, to simplify subsequent calculations. + */ + if ( align < padding ) + align = padding; - /* Allocate buffer plus descriptor as a single unit, unless - * doing so will push the total size over the alignment - * boundary. + /* Round up alignment to the nearest power of two, avoiding + * a potentially undefined shift operation. */ - if ( ( len + sizeof ( *iobuf ) ) <= align ) { + align_log2 = fls ( align - 1 ); + if ( align_log2 >= ( 8 * sizeof ( align ) ) ) + return NULL; + align = ( 1UL << align_log2 ); + + /* Calculate length threshold */ + assert ( align >= padding ); + threshold = ( align - padding ); + + /* Allocate buffer plus an inline descriptor as a single unit, + * unless doing so would push the total size over the + * alignment boundary. + */ + if ( len <= threshold ) { + + /* Round up buffer length to ensure that struct + * io_buffer is aligned. + */ + len += ( ( - len - offset ) & ( __alignof__ ( *iobuf ) - 1 ) ); /* Allocate memory for buffer plus descriptor */ data = malloc_dma_offset ( len + sizeof ( *iobuf ), align, |