Re: [PATCH v3] staging: r8188eu: Use strndup_user instead of kmalloc/copy_from_user
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-12-21 07:42:43
Also in:
lkml
On Mon, Dec 13, 2021 at 05:25:58PM +0800, Jiapeng Chong wrote:
quoted hunk ↗ jump to hunk
This code assumes that the user is going to give us a NUL terminated string which is not necessarily true. Fix following coccicheck warning: ./drivers/staging/r8188eu/os_dep/ioctl_linux.c:4253:8-15: WARNING opportunity for memdup_user. Reported-by: Abaci Robot <redacted> Fixes: 2b42bd58b321 ("staging: r8188eu: introduce new os_dep dir for RTL8188eu driver") Signed-off-by: Jiapeng Chong <redacted> --- Changes in v3: -Delete useless printk, modified the commit message and fixes tag. drivers/staging/r8188eu/os_dep/ioctl_linux.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c index 77728ba78d76..d8c28f279aa0 100644 --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c@@ -4247,18 +4247,10 @@ static int rtw_test( DBG_88E("+%s\n", __func__); len = wrqu->data.length; + pbuf = strndup_user(wrqu->data.pointer, len); + if (IS_ERR(pbuf)) + return PTR_ERR(pbuf); - pbuf = kzalloc(len, GFP_KERNEL); - if (!pbuf) { - DBG_88E("%s: no memory!\n", __func__); - return -ENOMEM; - } - - if (copy_from_user(pbuf, wrqu->data.pointer, len)) { - kfree(pbuf); - DBG_88E("%s: copy from user fail!\n", __func__); - return -EFAULT; - } DBG_88E("%s: string =\"%s\"\n", __func__, pbuf); ptmp = (char *)pbuf;-- 2.20.1.7.g153144c
This function does nothing, so it should just be deleted entirely. Please let's not have odd custom ioctls hanging around that can do nothing but cause problems. Also, this function looks like a lovely way to crash a kernel, even with your fix, so the sooner we get it removed the better. thanks, greg k-h