Commit f75da5af authored by Jens Axboe's avatar Jens Axboe

[PATCH] CDROM_SEND_PACKET bug

I just found Yet Another Bug in scsi_ioctl - CDROM_SEND_PACKET puts a
kernel pointer in hdr->cmdp, where sg_io() expects to find user address.
This worked up until recently because of the memcpy bug, but now it
doesn't because we do the proper copy_from_user(). 

This fix undoes the user copy code from sg_io, and instead makes the
SG_IO ioctl copy it locally.  This makes SG_IO and CDROM_SEND_PACKET
agree on the calling convention, and everybody is happy. 

I've tested that both

   cdrecord -dev=/dev/hdc -inq

and

   cdrecord -dev=ATAPI:/dev/hdc -inq

works now.  The former will use SG_IO, the latter CDROM_SEND_PACKET (and
incidentally would work in both 2.4 and 2.6, if it wasn't for
CDROM_SEND_PACKET sucking badly in 2.4).
parent 314dc154
...@@ -150,7 +150,6 @@ static int sg_io(request_queue_t *q, struct block_device *bdev, ...@@ -150,7 +150,6 @@ static int sg_io(request_queue_t *q, struct block_device *bdev,
struct request *rq; struct request *rq;
struct bio *bio; struct bio *bio;
char sense[SCSI_SENSE_BUFFERSIZE]; char sense[SCSI_SENSE_BUFFERSIZE];
unsigned char cdb[BLK_MAX_CDB];
void *buffer; void *buffer;
if (hdr->interface_id != 'S') if (hdr->interface_id != 'S')
...@@ -167,9 +166,6 @@ static int sg_io(request_queue_t *q, struct block_device *bdev, ...@@ -167,9 +166,6 @@ static int sg_io(request_queue_t *q, struct block_device *bdev,
if (hdr->dxfer_len > (q->max_sectors << 9)) if (hdr->dxfer_len > (q->max_sectors << 9))
return -EIO; return -EIO;
if (copy_from_user(cdb, hdr->cmdp, hdr->cmd_len))
return -EFAULT;
reading = writing = 0; reading = writing = 0;
buffer = NULL; buffer = NULL;
bio = NULL; bio = NULL;
...@@ -220,7 +216,7 @@ static int sg_io(request_queue_t *q, struct block_device *bdev, ...@@ -220,7 +216,7 @@ static int sg_io(request_queue_t *q, struct block_device *bdev,
* fill in request structure * fill in request structure
*/ */
rq->cmd_len = hdr->cmd_len; rq->cmd_len = hdr->cmd_len;
memcpy(rq->cmd, cdb, hdr->cmd_len); memcpy(rq->cmd, hdr->cmdp, hdr->cmd_len);
if (sizeof(rq->cmd) != hdr->cmd_len) if (sizeof(rq->cmd) != hdr->cmd_len)
memset(rq->cmd + hdr->cmd_len, 0, sizeof(rq->cmd) - hdr->cmd_len); memset(rq->cmd + hdr->cmd_len, 0, sizeof(rq->cmd) - hdr->cmd_len);
...@@ -436,12 +432,23 @@ int scsi_cmd_ioctl(struct block_device *bdev, unsigned int cmd, unsigned long ar ...@@ -436,12 +432,23 @@ int scsi_cmd_ioctl(struct block_device *bdev, unsigned int cmd, unsigned long ar
break; break;
case SG_IO: { case SG_IO: {
struct sg_io_hdr hdr; struct sg_io_hdr hdr;
unsigned char cdb[BLK_MAX_CDB], *old_cdb;
if (copy_from_user(&hdr, (struct sg_io_hdr *) arg, sizeof(hdr))) {
err = -EFAULT; err = -EFAULT;
if (copy_from_user(&hdr, (struct sg_io_hdr *) arg, sizeof(hdr)))
break; break;
} err = -EINVAL;
if (hdr.cmd_len > sizeof(rq->cmd))
break;
err = -EFAULT;
if (copy_from_user(cdb, hdr.cmdp, hdr.cmd_len))
break;
old_cdb = hdr.cmdp;
hdr.cmdp = cdb;
err = sg_io(q, bdev, &hdr); err = sg_io(q, bdev, &hdr);
hdr.cmdp = old_cdb;
if (copy_to_user((struct sg_io_hdr *) arg, &hdr, sizeof(hdr))) if (copy_to_user((struct sg_io_hdr *) arg, &hdr, sizeof(hdr)))
err = -EFAULT; err = -EFAULT;
break; break;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment