Messages in this thread Patch in this message | | | Date | Thu, 11 May 2006 13:21:36 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 1/4] Vectorize aio_read/aio_write methods |
| |
Badari Pulavarty <pbadari@us.ibm.com> wrote: > > On Thu, 2006-05-11 at 11:47 -0700, Andrew Morton wrote: > > Badari Pulavarty <pbadari@us.ibm.com> wrote: > > > > > > static ssize_t ep_aio_read_retry(struct kiocb *iocb) > > > { > > > struct kiocb_priv *priv = iocb->private; > > > - ssize_t status = priv->actual; > > > + ssize_t len, total; > > > > > > /* we "retry" to get the right mm context for this: */ > > > - status = copy_to_user(priv->ubuf, priv->buf, priv->actual); > > > - if (unlikely(0 != status)) > > > - status = -EFAULT; > > > - else > > > - status = priv->actual; > > > + > > > + /* copy stuff into user buffers */ > > > + total = priv->actual; > > > + len = 0; > > > + for (i=0; i < priv->count; i++) { > > > + ssize_t this = min(priv->iv[i].iov_len, total); > > > + > > > + if (copy_to_user(priv->iv[i].iov_buf, priv->buf, this)) > > > + break; > > > + > > > + total -= this; > > > + len += this; > > > + if (total <= 0) > > > + break; > > > + } > > > + > > > + if (unlikely(len == 0)) > > > + len = -EFAULT; > > > > This is still wrong, isn't it? Or am I looking at the same patch? > > > > There's no way in which `total' can go negative, so it'd be nicer to just > > test it for equality with zero. Because if it goes unexpectedly negative, > > we _want_ the kernel to malfunction, rather than mysteriously covering > > things up. > > > > The final test there should be > > > > if (unlikely(total != 0)) > > > > yes? > > No. The original check is correct - we want to return EFAULT if > copy_to_user() failed and we haven't copied anything so far. > If we copied anything so far, we should return, that many bytes. > (like short-io).
oic. And we're sure that we cannot call into this code if someone's trying a zero-sized read?
Either way, the below (which is faster!) will fix, yes?
--- 25/drivers/usb/gadget/inode.c~vectorize-aio_read-aio_write-methods-fix Thu May 11 11:53:41 2006 +++ 25-akpm/drivers/usb/gadget/inode.c Thu May 11 13:19:45 2006 @@ -567,18 +567,18 @@ static ssize_t ep_aio_read_retry(struct for (i = 0; i < priv->count; i++) { ssize_t this = min(priv->iv[i].iov_len, total); - if (copy_to_user(priv->iv[i].iov_buf, priv->buf, this)) + if (copy_to_user(priv->iv[i].iov_buf, priv->buf, this)) { + if (len == 0) + len = -EFAULT; break; + } total -= this; len += this; - if (total <= 0) + if (total == 0) break; } - if (unlikely(len == 0)) - len = -EFAULT; - kfree(priv->buf); kfree(priv); aio_put_req(iocb); _ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |