From: "Robert T. Johnson" <rtjohnso@eecs.berkeley.edu>
To: linux-scsi@vger.kernel.org
Cc: Linux Kernel <linux-kernel@vger.kernel.org>
Subject: PATCH: 2.6.7-rc3 drivers/scsi/cpqfcTSinit.c several user/kernel pointer bugs
Date: 09 Jun 2004 15:23:34 -0700 [thread overview]
Message-ID: <1086819814.14180.86.camel@dooby.cs.berkeley.edu> (raw)
Since "ioc" is copied from user-space, vendor_cmd = ioc.argp cannot be trusted,
so derefing vendor_cmd via expressions like "vendor_cmd->len" is unsafe.
Let me know if you have any questions, and I apologize if I've made a mistake
in my analysis.
Best,
Rob
--- linux-2.6.7-rc3-full/drivers/scsi/cpqfcTSinit.c.orig Wed Jun 9 12:20:15 2004
+++ linux-2.6.7-rc3-full/drivers/scsi/cpqfcTSinit.c Wed Jun 9 12:18:50 2004
@@ -556,7 +556,7 @@ int cpqfcTS_ioctl( struct scsi_device *S
struct scsi_cmnd *DumCmnd;
int i, j;
VENDOR_IOCTL_REQ ioc;
- cpqfc_passthru_t *vendor_cmd;
+ cpqfc_passthru_t vendor_cmd;
Scsi_Device *SDpnt;
Scsi_Request *ScsiPassThruReq;
cpqfc_passthru_private_t *privatedata;
@@ -590,12 +590,14 @@ int cpqfcTS_ioctl( struct scsi_device *S
if( copy_from_user( &ioc, arg, sizeof( VENDOR_IOCTL_REQ)))
return( -EFAULT);
- vendor_cmd = ioc.argp; // i.e., CPQ specific command struct
+ // CPQ specific command struct
+ if (copy_from_user(&vendor_cmd, ioc.argp, sizeof(vendor_cmd)))
+ return (-EFAULT);
// If necessary, grab a kernel/DMA buffer
- if( vendor_cmd->len)
+ if( vendor_cmd.len)
{
- buf = kmalloc( vendor_cmd->len, GFP_KERNEL);
+ buf = kmalloc( vendor_cmd.len, GFP_KERNEL);
if( !buf)
return -ENOMEM;
}
@@ -613,10 +615,10 @@ int cpqfcTS_ioctl( struct scsi_device *S
return -ENOMEM;
}
- if (vendor_cmd->rw_flag == VENDOR_WRITE_OPCODE) {
- if (vendor_cmd->len) { // Need data from user?
- if (copy_from_user(buf, vendor_cmd->bufp,
- vendor_cmd->len)) {
+ if (vendor_cmd.rw_flag == VENDOR_WRITE_OPCODE) {
+ if (vendor_cmd.len) { // Need data from user?
+ if (copy_from_user(buf, vendor_cmd.bufp,
+ vendor_cmd.len)) {
kfree(buf);
cpqfc_free_private_data(cpqfcHBAdata,
ScsiPassThruReq->upper_private_data);
@@ -625,7 +627,7 @@ int cpqfcTS_ioctl( struct scsi_device *S
}
}
ScsiPassThruReq->sr_data_direction = SCSI_DATA_WRITE;
- } else if (vendor_cmd->rw_flag == VENDOR_READ_OPCODE) {
+ } else if (vendor_cmd.rw_flag == VENDOR_READ_OPCODE) {
ScsiPassThruReq->sr_data_direction = SCSI_DATA_READ;
} else
// maybe this means a bug in the user app
@@ -642,12 +644,12 @@ int cpqfcTS_ioctl( struct scsi_device *S
// FC Link events with our "worker thread".
privatedata = ScsiPassThruReq->upper_private_data;
- privatedata->bus = vendor_cmd->bus;
- privatedata->pdrive = vendor_cmd->pdrive;
+ privatedata->bus = vendor_cmd.bus;
+ privatedata->pdrive = vendor_cmd.pdrive;
// eventually gets us to our own _quecommand routine
scsi_wait_req(ScsiPassThruReq,
- &vendor_cmd->cdb[0], buf, vendor_cmd->len,
+ &vendor_cmd.cdb[0], buf, vendor_cmd.len,
10*HZ, // timeout
1); // retries
result = ScsiPassThruReq->sr_result;
@@ -655,12 +657,12 @@ int cpqfcTS_ioctl( struct scsi_device *S
// copy any sense data back to caller
if( result != 0 )
{
- memcpy( vendor_cmd->sense_data, // see struct def - size=40
+ memcpy( vendor_cmd.sense_data, // see struct def - size=40
ScsiPassThruReq->sr_sense_buffer,
sizeof(ScsiPassThruReq->sr_sense_buffer) <
- sizeof(vendor_cmd->sense_data) ?
+ sizeof(vendor_cmd.sense_data) ?
sizeof(ScsiPassThruReq->sr_sense_buffer) :
- sizeof(vendor_cmd->sense_data)
+ sizeof(vendor_cmd.sense_data)
);
}
SDpnt = ScsiPassThruReq->sr_device;
@@ -669,9 +671,9 @@ int cpqfcTS_ioctl( struct scsi_device *S
ScsiPassThruReq = NULL;
// need to pass data back to user (space)?
- if( (vendor_cmd->rw_flag == VENDOR_READ_OPCODE) &&
- vendor_cmd->len )
- if( copy_to_user( vendor_cmd->bufp, buf, vendor_cmd->len))
+ if( (vendor_cmd.rw_flag == VENDOR_READ_OPCODE) &&
+ vendor_cmd.len )
+ if( copy_to_user( vendor_cmd.bufp, buf, vendor_cmd.len))
result = -EFAULT;
if( buf)
reply other threads:[~2004-06-09 22:23 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1086819814.14180.86.camel@dooby.cs.berkeley.edu \
--to=rtjohnso@eecs.berkeley.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox