On Wed, Oct 02, 2019 at 02:37:20PM +0200, Marek Szyprowski wrote:
malloc_cache_aligned() might return zero, so fix potential NULL pointer access if __GFP_ZERO flag is set.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Ralph Siemsen ralph.siemsen@linaro.org
This looks reasonable to me. The memset() will happily scribble all over memory at address zero, which can be very hard to track down later on. So adding a check seems prudent. Note that I am not a maintainer ;-)
lib/linux_compat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/linux_compat.c b/lib/linux_compat.c index 6373b4451e..81ea8fb126 100644 --- a/lib/linux_compat.c +++ b/lib/linux_compat.c @@ -20,7 +20,7 @@ void *kmalloc(size_t size, int flags) void *p;
p = malloc_cache_aligned(size);
- if (flags & __GFP_ZERO)
if (p && flags & __GFP_ZERO) memset(p, 0, size);
return p;
-- 2.17.1
Perhaps I can hijack this thread slightly: have others encountered problems due to the use of malloc_cache_aligned() in kmalloc() and kmem_cache_alloc()? This substantially increases memory usage, in particular for UBI which allocates two small structures for every physical erase block. This adds up quickly on large flash chips.
I understand the rationale for padding the allocation, as explained in commit e3332e1a1a04534225801c2710c6faef4809641c. However, it also seems that in Linux, allocations are aligned/padded only when certain flags are set (GFP_DMA or GFP_DMA32 in particular). These flags are not currently defined in u-boot.
For UBI at least, there is no need to align or pad the structures, they are not accessed by DMA. So it would be good if we could avoid memory overhead. Possible options would include:
- go back to plain memalign() or even just plain malloc(), with the caveat that callers who need specific alignment must handle it themselves (simplest option, no overhead) - add the missing flags, and check them at runtime. (adding code and some overhead) - hybrid versions, such as checking flags on kmalloc, but not doing any padding/alignment for kmem_cache_alloc
I would be happy to work on patches for this, in a separate thread of course. Would be helpful to know which option would be acceptable.
Cheers, Ralph