Messages in this thread | | | Date | Wed, 22 Apr 2020 03:58:40 +0100 | From | Al Viro <> | Subject | Re: [PATCH -next] ide: Use memdup_user() as a cleanup |
| |
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.
| |