Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] udmabuf: order-10 contiguous allocation in udmabuf_ioctl_create_list() - [Low] udmabuf: missing kernel-parameters.txt update for list_limit parameter --
commit 70d859f4b270b1a12dc2cb5cc01faf794efe4700 Author: Val Packett <[email protected]> udmabuf: Increase the default list length limit This commit increases the default list_limit module parameter in drivers/dma-buf/udmabuf.c from 1024 to 131072 to support larger page lists for fragmented guest memory buffers. > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 6fb69d8399c18..9aec0e3a1e49b 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -16,9 +16,9 @@ > #include <linux/vmalloc.h> > #include <linux/iosys-map.h> > > -static int list_limit = 1024; > +static int list_limit = 131072; [Severity: Medium] With list_limit raised to 131072, udmabuf_ioctl_create_list() in drivers/dma-buf/udmabuf.c copies the user item array using memdup_array_user(): if (head.count > list_limit) return -EINVAL; list = memdup_array_user((void __user *)(arg + sizeof(head)), head.count, sizeof(struct udmabuf_create_item)); if (IS_ERR(list)) return PTR_ERR(list); ret = udmabuf_create(filp->private_data, &head, list); kfree(list); which calls memdup_user() in mm/util.c: p = kmem_buckets_alloc_track_caller(user_buckets, len, GFP_USER | __GFP_NOWARN); if (!p) return ERR_PTR(-ENOMEM); Each struct udmabuf_create_item is 24 bytes, so a head.count of 80,000 to 131,072 entries requests roughly 1.83 MiB to 3 MiB (order 9 to order 10 on 4 KiB page systems). Because memdup_array_user() -> memdup_user() -> kmem_buckets_alloc_track_caller() allocates physically contiguous memory without a vmalloc fallback, could this fail with -ENOMEM or stall in direct compaction on hosts with fragmented physical memory (or fail unconditionally when the maximum page allocation order is less than 10)? Should udmabuf_ioctl_create_list() use vmemdup_array_user() and kvfree() for list instead? > module_param(list_limit, int, 0644); > -MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is > 1024."); > +MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is > 131072."); [Severity: Low] Since list_limit in drivers/dma-buf/udmabuf.c is exposed as a kernel command-line parameter via module_param(list_limit, int, 0644) (as udmabuf.list_limit), should Documentation/admin-guide/kernel-parameters.txt also be updated to document this parameter and its new default? > > static int size_limit_mb = INT_MAX; > module_param(size_limit_mb, int, 0644); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
