Re: [PATCH 1/5] sgl_alloc_order: remove 4 GiB limit

[Jason Gunthorpe <jgg@ziepe.ca>]

On Sun, Oct 23, 2022 at 09:02:40PM -0400, Douglas Gilbert wrote:
> This patch fixes a check done by sgl_alloc_order() before it starts
> any allocations. The comment in the original said: "Check for integer
> overflow" but the right hand side of the expression in the condition
> is resolved as u32 so it can not exceed UINT32_MAX (4 GiB) which
> means 'length' can not exceed that value.
> 
> This function may be used to replace vmalloc(unsigned long) for a
> large allocation (e.g. a ramdisk). vmalloc has no limit at 4 GiB so
> it seems unreasonable that sgl_alloc_order() whose length type is
> unsigned long long should be limited to 4 GB.
> 
> Solutions to this issue were discussed by Jason Gunthorpe
> <jgg@ziepe.ca> and Bodo Stroesser <bostroesser@gmail.com>. This
> version is base on a linux-scsi post by Jason titled: "Re:
> [PATCH v7 1/4] sgl_alloc_order: remove 4 GiB limit" dated 20220201.

You should link to lore here..

I think I prefer we just fix this so it doesn't have a problem in the
first place - nothing needs the weird unsigned long long argument type:

diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c8c3d675845c37..c39e19fa174bca 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -595,19 +595,17 @@ EXPORT_SYMBOL(sg_alloc_table_from_pages_segment);
  *
  * Returns: A pointer to an initialized scatterlist or %NULL upon failure.
  */
-struct scatterlist *sgl_alloc_order(unsigned long long length,
-				    unsigned int order, bool chainable,
-				    gfp_t gfp, unsigned int *nent_p)
+struct scatterlist *sgl_alloc_order(size_t length, unsigned int order,
+				    bool chainable, gfp_t gfp, size_t *nent_p)
 {
 	struct scatterlist *sgl, *sg;
 	struct page *page;
-	unsigned int nent, nalloc;
+	size_t nent, nalloc;
 	u32 elem_len;
 
-	nent = round_up(length, PAGE_SIZE << order) >> (PAGE_SHIFT + order);
-	/* Check for integer overflow */
-	if (length > (nent << (PAGE_SHIFT + order)))
-		return NULL;
+	nent = length >> (PAGE_SHIFT + order);
+	if (length % (1 << (PAGE_SHIFT + order)))
+		nent++;
 	nalloc = nent;
 	if (chainable) {
 		/* Check for integer overflow */
@@ -649,8 +647,7 @@ EXPORT_SYMBOL(sgl_alloc_order);
  *
  * Returns: A pointer to an initialized scatterlist or %NULL upon failure.
  */
-struct scatterlist *sgl_alloc(unsigned long long length, gfp_t gfp,
-			      unsigned int *nent_p)
+struct scatterlist *sgl_alloc(size_t length, gfp_t gfp, size_t *nent_p)
 {
 	return sgl_alloc_order(length, 0, false, gfp, nent_p);
 }

Jason