On Fri, Nov 18, 2005 at 10:01:24PM +0100, Jens Axboe wrote:
> On Fri, Nov 18 2005, mikem wrote:
> > Patch 2 of 3
> >
> > Applications using CCISS_BIG_PASSTHRU complained that the data written
> > was zeros. The code looked alright, but it seems that copy_from_user
> > already does a memset on the buffer. Removing it from the pass-through
> > fixes the apps.
>
> Hmm, I don't like this patch, since you never clear the buffer for reads
> now. If the controller for some reason doesn't overwrite this buffer,
> you could be leaking privileged data! Your bug is because you do:
>
> if (write && copy_from_user(...))
> fail
> else
> clear
>
> so you end up in the clear case for any case where copy_from_user()
> doesn't fail. I've fixed it up for you, this is what I committed:
Thanks, Jens.
>
> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> index e239a6c..33f8341 100644
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -1017,10 +1017,11 @@ static int cciss_ioctl(struct inode *ino
> status = -ENOMEM;
> goto cleanup1;
> }
> - if (ioc->Request.Type.Direction == XFER_WRITE &&
> - copy_from_user(buff[sg_used], data_ptr, sz)) {
> + if (ioc->Request.Type.Direction == XFER_WRITE) {
> + if (copy_from_user(buff[sg_used], data_ptr, sz)) {
> status = -ENOMEM;
> - goto cleanup1;
> + goto cleanup1;
> + }
> } else {
> memset(buff[sg_used], 0, sz);
> }
>
> --
> Jens Axboe
>
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
