On 2/18/24 11:46, Christophe JAILLET wrote:
If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough.
In such a case, udmabuf_create() would access to memory beyond 'list'.
Use memdup_array_user() which checks for overflow.
While at it, include <linux/string.h>.
Fixes: fbb0de795078 ("Add udmabuf misc device")'
I don't think this tag is needed in this case.
Also, please, CC linux-hardening next time.
Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr
In any case, LGTM:
Reviewed-by: Gustavo A. R. Silva gustavoars@kernel.org
Thanks! -- Gustavo
v2: - Use memdup_array_user() [Kees Cook] - Use sizeof(*list) [Gustavo A. R. Silva] - Add include <linux/string.h>
v1: https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.1695060...
Signed-off-by: Christophe JAILLET christophe.jaillet@wanadoo.fr
drivers/dma-buf/udmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..5728948ea6f2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -11,6 +11,7 @@ #include <linux/module.h> #include <linux/shmem_fs.h> #include <linux/slab.h> +#include <linux/string.h> #include <linux/udmabuf.h> #include <linux/vmalloc.h> #include <linux/iosys-map.h> @@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL;
- u32 lsize;
if (copy_from_user(&head, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
- list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
- list = memdup_array_user((void __user *)(arg + sizeof(head)),
sizeof(*list), head.count);