| Message ID | 1587524381-120136-1-git-send-email-zou_wei@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [-next] ide: Use memdup_user() as a cleanup | expand |
On Wed, Apr 22, 2020 at 10:59:41AM +0800, Zou Wei wrote: > if (taskout) { > int outtotal = tasksize; > - outbuf = kzalloc(taskout, GFP_KERNEL); > - if (outbuf == NULL) { > - err = -ENOMEM; > - goto abort; > - } > - if (copy_from_user(outbuf, buf + outtotal, taskout)) { > - err = -EFAULT; > - goto abort; > - } > + outbuf = memdup_user(buf + outtotal, taskout); > + if (IS_ERR(outbuf)) > + return PTR_ERR(outbuf); > } > > if (taskin) { > int intotal = tasksize + taskout; > - inbuf = kzalloc(taskin, GFP_KERNEL); > - if (inbuf == NULL) { > - err = -ENOMEM; > - goto abort; > - } > - if (copy_from_user(inbuf, buf + intotal, taskin)) { > - err = -EFAULT; > - goto abort; > - } > + inbuf = memdup_user(buf + intotal, taskin); > + if (IS_ERR(inbuf)) > + return PTR_ERR(inbuf); That smells like a leak - what happens if both taskin and taskout are non-zero at the same time? <looks> actually, both parts are leaking - there's req_task = memdup_user(buf, tasksize); if (IS_ERR(req_task)) return PTR_ERR(req_task); shortly prior to that, so both of your failure exits are leaking.
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c index aab6a10..336b575 100644 --- a/drivers/ide/ide-taskfile.c +++ b/drivers/ide/ide-taskfile.c @@ -489,28 +489,16 @@ int ide_taskfile_ioctl(ide_drive_t *drive, unsigned long arg) if (taskout) { int outtotal = tasksize; - outbuf = kzalloc(taskout, GFP_KERNEL); - if (outbuf == NULL) { - err = -ENOMEM; - goto abort; - } - if (copy_from_user(outbuf, buf + outtotal, taskout)) { - err = -EFAULT; - goto abort; - } + outbuf = memdup_user(buf + outtotal, taskout); + if (IS_ERR(outbuf)) + return PTR_ERR(outbuf); } if (taskin) { int intotal = tasksize + taskout; - inbuf = kzalloc(taskin, GFP_KERNEL); - if (inbuf == NULL) { - err = -ENOMEM; - goto abort; - } - if (copy_from_user(inbuf, buf + intotal, taskin)) { - err = -EFAULT; - goto abort; - } + inbuf = memdup_user(buf + intotal, taskin); + if (IS_ERR(inbuf)) + return PTR_ERR(inbuf); } memset(&cmd, 0, sizeof(cmd));
Fix coccicheck warning which recommends to use memdup_user(). This patch fixes the following coccicheck warnings: drivers/ide/ide-taskfile.c:492:11-18: WARNING opportunity for memdup_user drivers/ide/ide-taskfile.c:505:10-17: WARNING opportunity for memdup_user Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zou Wei <zou_wei@huawei.com> --- drivers/ide/ide-taskfile.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-)