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 - 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
