Re: [PATCH 4/6] lib/scatterlist: Do not leak pages when high-order allocation fails

[Bart Van Assche]

On Wed, 2018-09-26 at 15:16 +-0100, Tvrtko Ursulin wrote:
+AD4 From: Tvrtko Ursulin +ADw-tvrtko.ursulin+AEA-intel.com+AD4
+AD4 
+AD4 If a higher-order allocation fails, the existing abort and cleanup path
+AD4 would consider all segments allocated so far as 0-order page allocations
+AD4 and would therefore leak memory.
+AD4 
+AD4 Fix this by cleaning up using sgl+AF8-free+AF8-n+AF8-order which allows the correct
+AD4 page order to be passed in.
+AD4 
+AD4 Signed-off-by: Tvrtko Ursulin +ADw-tvrtko.ursulin+AEA-intel.com+AD4
+AD4 Cc: Bart Van Assche +ADw-bart.vanassche+AEA-wdc.com+AD4
+AD4 Cc: Hannes Reinecke +ADw-hare+AEA-suse.com+AD4
+AD4 Cc: Johannes Thumshirn +ADw-jthumshirn+AEA-suse.de+AD4
+AD4 Cc: Jens Axboe +ADw-axboe+AEA-kernel.dk+AD4
+AD4 ---
+AD4  lib/scatterlist.c +AHw 6 +-+-+-+---
+AD4  1 file changed, 4 insertions(+-), 2 deletions(-)
+AD4 
+AD4 diff --git a/lib/scatterlist.c b/lib/scatterlist.c
+AD4 index 3cc01cd82242..0caed79d7291 100644
+AD4 --- a/lib/scatterlist.c
+AD4 +-+-+- b/lib/scatterlist.c
+AD4 +AEAAQA -481,7 +-481,7 +AEAAQA struct scatterlist +ACo-sgl+AF8-alloc+AF8-order(unsigned long length, unsigned int order,
+AD4  +AHs
+AD4  	struct scatterlist +ACo-sgl, +ACo-sg+ADs
+AD4  	struct page +ACo-page+ADs
+AD4 -	unsigned int nent, nalloc+ADs
+AD4 +-	unsigned int nent, nalloc, i+ADs
+AD4  	u32 elem+AF8-len+ADs
+AD4  
+AD4  	nent +AD0 round+AF8-up(length, PAGE+AF8-SIZE +ADwAPA order) +AD4APg (PAGE+AF8-SHIFT +- order)+ADs
+AD4 +AEAAQA -501,17 +-501,19 +AEAAQA struct scatterlist +ACo-sgl+AF8-alloc+AF8-order(unsigned long length, unsigned int order,
+AD4  
+AD4  	sg+AF8-init+AF8-table(sgl, nalloc)+ADs
+AD4  	sg +AD0 sgl+ADs
+AD4 +-	i +AD0 0+ADs
+AD4  	while (length) +AHs
+AD4  		elem+AF8-len +AD0 min+AF8-t(u64, length, PAGE+AF8-SIZE +ADwAPA order)+ADs
+AD4  		page +AD0 alloc+AF8-pages(gfp, order)+ADs
+AD4  		if (+ACE-page) +AHs
+AD4 -			sgl+AF8-free(sgl)+ADs
+AD4 +-			sgl+AF8-free+AF8-n+AF8-order(sgl, i, order)+ADs
+AD4  			return NULL+ADs
+AD4  		+AH0
+AD4  
+AD4  		sg+AF8-set+AF8-page(sg, page, elem+AF8-len, 0)+ADs
+AD4  		length -+AD0 elem+AF8-len+ADs
+AD4  		sg +AD0 sg+AF8-next(sg)+ADs
+AD4 +-		i+-+-+ADs
+AD4  	+AH0
+AD4  	WARN+AF8-ONCE(length, +ACI-length +AD0 +ACU-ld+AFw-n+ACI, length)+ADs
+AD4  	if (nent+AF8-p)

sg+AF8-init+AF8-table() clears all page pointers and sgl+AF8-free+AF8-n+AF8-order() can handle
elements of which the page pointer is zero. So I think if
sgl+AF8-free+AF8-n+AF8-order(sgl, i, order) would be changed into sgl+AF8-free+AF8-n+AF8-order(sgl,
UINT+AF8-MAX, order) that the variable 'i' can be left out.

Bart.