> -----Original Message----- > From: linux-scsi-owner@xxxxxxxxxxxxxxx > [mailto:linux-scsi-owner@xxxxxxxxxxxxxxx] On Behalf Of Boaz Harrosh > Sent: Sunday, July 01, 2007 6:56 AM > To: Ed Lin; FUJITA Tomonori > Cc: g.liakhovetski@xxxxxx; linux-scsi@xxxxxxxxxxxxxxx; James Bottomley > Subject: Re: [PATCH 3/3] Farther clean up of stex.c driver > > > Ed Lin wrote: > > > >>>> > >>> The software allocates a big enough buffer(so don't worry > about this), > >>> and it trims the data size based on returned length. So > it needs the > >>> actual data length. > > This sounds like a security breach to me. An untrusted user-mode > with knowledge of such hardware in the machine can gain > access to kernel > through the wrong( cracker-right) sized request. > > >> What's the software? I mean that who trims the data size based on > >> returned length? > >> > > > > The management software provided by Promise. > > > > I had a deeper look into this, and I am totally confused. > The code in question at stex.c::stex_ys_commands() is called > at softirq from stex_mu_intr(). It will than immediately go > on to eventually call cmd->scsi_done(cmd); > Now what is the upper layer that calls this command??! > > There can be 3 possibilities that I know. From unlikely to > likely order: > 1. REQ_TYPE_FS command which is not possible because I don't know > of any one that can issue. a ccb->cmd->cmnd[0] == MGT_CMD > as an REQ_TYPE_FS. And anyway if bufflen is truncated like that > a REQ_TYPE_FS command will retry the command until complete. > > 2. REQ_TYPE_BLOCK_PC through sg, bsg or others at which truncating > a command like that will leak BIOs when trying to release the > command, since scsi-ml relies on bufflen in the call to > end_that_request_first. > > 3. A special upper layer that overrides the done functions at > scsi_cmnd or request levels and interprets the return info > and/or frees buffers. If this is the case, can such a driver > use a reserved member of scsi_cmnd like the fields in > struct scsi_pointer or something hanging on host_scribble? > Than later return to user-mode what it used to return before > but leave bufflen alone? > > Setting of bufflen like that in a driver sounds like a highway > rubbery to me. Please stop that Hack. Now! > > >>> So this length fix is necessary if software doesn't > >>> change policy. The whole thing is guaranteed by both software and > >>> firmware, software will do a check, firmware will do a check, > >>> so a check in driver is not necessary. > > > As I said. I'm sure we can change upper layer polices to not > break user-mode but to also not touch a delicate and private member > of scsi-ml. It limits the scsi-ml ability to change and evolve. > The most I can do for now is declare this driver broken once > scsi_sgtable goes in because I sure am not going to do: > ccb->cmd->sg_table->length = le32_to_cpu(*(__le32 > *)&resp->variable[0]); > (Do we need an explanation why this is plain broken, leaking code) > > If you point me to the upper layer driver used, I can see if > I can change it > in a way that will not touch bufflen. > > Another thing I do not understand is where in the SCSI standard > such an operation is defined. because if it is a standard SCSI > command or extended or vendor-specific command than, current code > is not built to cope with such scenario. How would one defined > it? a "truncated read"? > > Boaz Harrosh > We are investigating it. Please wait a while. We are also trying to make this driver better. Thanks for your findings and efforts. Ed Lin - To unsubscribe from this list: 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
