Read/Write Buffer support
RFC 1/1 Read/Write Buffer support
This patch adds partial support for the SCSI commands READ_BUFFER and WRITE_BUFFER. Because ATA only supports a single 512-byte buffer per drive, this isn't as useful as it could be, but it might prove interesting when extended to handle the DOWNLOAD MICROCODE operations.
Comments?
drivers/ata/libata-scsi.c +115 -0
@@ -1711,6 +1711,117 @@ void ata_scsi_rbuf_fill(struct ata_scsi_args *args, args->done (cmd); } +/* +* This helper is used for three subcommands of READ_BUFFER. Subcommand 0 +* wants a header prefixing the data. Subcommand 3 returns a descriptor +* and subcommand 0xb returns a descriptor for the echo buffer. While these +* all have slightly different formats, in practise they all look sufficiently +* similar that this common function can be used. +*/ +static void ata_scsi_fill_buffer_descriptor (struct scsi_cmnd *cmd,int desc) +{ +u8 *rbuf; +unsigned int buflen; +unsigned long flags; + +local_irq_save (flags); + + buflen =ata_scsi_rbuf_get (cmd, &rbuf); + +memset (rbuf,0 , buflen); + rbuf[0 ] = desc ?0xff :0x0 ; + rbuf[1 ] =0x0 ; + rbuf[2 ] =0x2 ; + rbuf[3 ] =0x0 ; + +ata_scsi_rbuf_put (cmd, rbuf); + +local_irq_restore (flags); +} + +static int ata_scsi_read_buffer_descriptor (struct scsi_cmnd *cmd) +{ +ata_scsi_fill_buffer_descriptor (cmd, cmd->cmnd[1 ] ==0x3 ); + + cmd->result =SAM_STAT_GOOD ; +return 1 ; +} + +static void ata_scsi_rwbuf_header (struct scsi_cmnd *cmd) +{ +struct scatterlist *sg =scsi_sglist (cmd); +ata_scsi_fill_buffer_descriptor (cmd,0 ); + +/* XXX: fewer than 4 bytes in the page? Doomed. */ + sg->offset +=4 ; + sg->length -=4 ; +} + +/** +* ata_scsi_rwbuf_xlat - Translate READ_BUFFER and WRITE_BUFFER +* +* SCSI has a very flexible READ_BUFFER and WRITE_BUFFER system. +* ATA's READ BUFFER only supports an offset of 0 and length 512. +* SCSI also uses WRITE_BUFFER for DOWNLOAD MICROCODE, which is not +* yet supported by this function. +*/ +static unsigned int ata_scsi_rwbuf_xlat (struct ata_queued_cmd *qc) +{ +struct scsi_cmnd *scmd = qc->scsicmd; +const u8 *cdb = scmd->cmnd; +struct ata_taskfile *tf = &qc->tf; +int offset =0 ; + +if (cdb[0 ] ==WRITE_BUFFER ) { + tf->flags |=ATA_TFLAG_WRITE ; + tf->command =ATA_CMD_WRITE_BUFFER ; + }else { + tf->command =ATA_CMD_READ_BUFFER ; + } + +switch (cdb[1 ]) { +case 0x0 :/* Read/write data with header */ +ata_scsi_rwbuf_header (scmd); + offset =4 ; +case 0x2 :/* Read/write data */ +case 0xa :/* Read/write echo buffer */ +break ; +case 0x3 :/* Read descriptor */ +case 0xb :/* Read echo buffer descriptor */ +if (cdb[0 ] ==READ_BUFFER ) +return ata_scsi_read_buffer_descriptor (scmd); +case 0x4 : +case 0x5 : +case 0x6 : +case 0x7 : +/* XXX: We can translate to DOWNLOAD_MICROCODE here */ + +default : + goto invalid_fld; + } + +if ((cdb[2 ] !=0 ) || (cdb[3 ] !=0 ) || (cdb[4 ] !=0 ) || (cdb[5 ] !=0 )) + goto invalid_fld; +if (((cdb[6 ] <<16 ) | (cdb[7 ] <<8 ) | cdb[8 ]) != (512 + offset)) + goto invalid_fld; + + qc->flags |=ATA_QCFLAG_IO ; + qc->nbytes =512 ; + tf->flags |=ATA_TFLAG_ISADDR |ATA_TFLAG_DEVICE ; + tf->protocol =ATA_PROT_PIO ; + tf->nsect =0 ; + tf->lbah =0 ; + tf->lbam =0 ; + tf->lbal =0 ; + +return 0 ; + +invalid_fld: +ata_scsi_set_sense (scmd,ILLEGAL_REQUEST ,0x24 ,0x0 ); +/* "Invalid field in cbd" */ +return 1 ; +} +/** * ATA_SCSI_RBUF_SET - helper to set values in SCSI response buffer * @idx: byte index into SCSI response buffer
@@ -2913,6 +3024,10 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)case WRITE_16 :return ata_scsi_rw_xlat; +case READ_BUFFER : +case WRITE_BUFFER : +return ata_scsi_rwbuf_xlat; +case SYNCHRONIZE_CACHE :if (ata_try_flush_cache (dev))return ata_scsi_flush_xlat;
include/linux/ata.h +2 -0
@@ -176,6 +176,8 @@ enum {ATA_CMD_WRITE_MULTI_FUA_EXT =0xCE ,ATA_CMD_SET_FEATURES =0xEF ,ATA_CMD_SET_MULTI =0xC6 , +ATA_CMD_READ_BUFFER =0xE4 , +ATA_CMD_WRITE_BUFFER =0xE8 ,ATA_CMD_PACKET =0xA0 ,ATA_CMD_VERIFY =0x40 , ATA_CMD_VERIFY_EXT = 0x42,
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 4 lines · Matthew Wilcox
This patch adds partial support for the SCSI commands
READ_BUFFERandWRITE_BUFFER. Because ATA only supports a single 512-byte buffer per drive, this isn't as useful as it could be, but it might prove interesting when extended to handle the DOWNLOAD MICROCODE operations.
General concept: ACK
Minor nits below...
Quoted 25 lines
drivers/ata/libata-scsi.c +17
-0@@ -1711,6 +1711,117 @@ void ata_scsi_rbuf_fill(struct ata_scsi_args *args, args->done (cmd); } +/* +* This helper is used for three subcommands of READ_BUFFER. Subcommand 0 +* wants a header prefixing the data. Subcommand 3 returns a descriptor +* and subcommand 0xb returns a descriptor for the echo buffer. While these +* all have slightly different formats, in practise they all look sufficiently +* similar that this common function can be used. +*/ +static void ata_scsi_fill_buffer_descriptor (struct scsi_cmnd *cmd,int desc) +{ +u8 *rbuf; +unsigned int buflen; +unsigned long flags; + +local_irq_save (flags); + + buflen =ata_scsi_rbuf_get (cmd, &rbuf); +
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1740
Quoted 10 lines
drivers/ata/libata-scsi.c +10
-0+memset (rbuf,0 , buflen); + rbuf[0 ] = desc ?0xff :0x0 ; + rbuf[1 ] =0x0 ; + rbuf[2 ] =0x2 ; + rbuf[3 ] =0x0 ; + +ata_scsi_rbuf_put (cmd, rbuf); + +local_irq_restore (flags); +}
use ata_scsi_rbuf_fill() rather than recreating it manually
Quoted 6 lines
drivers/ata/libata-scsi.c +6
-0+static int ata_scsi_read_buffer_descriptor (struct scsi_cmnd *cmd) +{ +ata_scsi_fill_buffer_descriptor (cmd, cmd->cmnd[1 ] ==0x3 ); + + cmd->result =SAM_STAT_GOOD ; +return 1 ;
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1757
Quoted 10 lines
drivers/ata/libata-scsi.c +10
-0+} + +static void ata_scsi_rwbuf_header (struct scsi_cmnd *cmd) +{ +struct scatterlist *sg =scsi_sglist (cmd); +ata_scsi_fill_buffer_descriptor (cmd,0 ); + +/* XXX: fewer than 4 bytes in the page? Doomed. */ + sg->offset +=4 ; + sg->length -=4 ;
I'm quite uncomfortable skipping any amount of validation.
Quoted 28 lines
drivers/ata/libata-scsi.c +28
-0+/** +* ata_scsi_rwbuf_xlat - Translate READ_BUFFER and WRITE_BUFFER +* +* SCSI has a very flexible READ_BUFFER and WRITE_BUFFER system. +* ATA's READ BUFFER only supports an offset of 0 and length 512. +* SCSI also uses WRITE_BUFFER for DOWNLOAD MICROCODE, which is not +* yet supported by this function. +*/ +static unsigned int ata_scsi_rwbuf_xlat (struct ata_queued_cmd *qc) +{ +struct scsi_cmnd *scmd = qc->scsicmd; +const u8 *cdb = scmd->cmnd; +struct ata_taskfile *tf = &qc->tf; +int offset =0 ; + +if (cdb[0 ] ==WRITE_BUFFER ) { + tf->flags |=ATA_TFLAG_WRITE ; + tf->command =ATA_CMD_WRITE_BUFFER ; + }else { + tf->command =ATA_CMD_READ_BUFFER ; + } + +switch (cdb[1 ]) { +case 0x0 :/* Read/write data with header */ +ata_scsi_rwbuf_header (scmd); + offset =4 ; +case 0x2 :/* Read/write data */ +case 0xa :/* Read/write echo buffer */
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1797
Quoted 10 lines
drivers/ata/libata-scsi.c +10
-0+break ; +case 0x3 :/* Read descriptor */ +case 0xb :/* Read echo buffer descriptor */ +if (cdb[0 ] ==READ_BUFFER ) +return ata_scsi_read_buffer_descriptor (scmd); +case 0x4 : +case 0x5 : +case 0x6 : +case 0x7 : +/* XXX: We can translate to DOWNLOAD_MICROCODE here */
Style in libata is "FIXME" or "TODO" not "XXX". Please help with consistency, it makes grepping easier.
Plus, I dunno about you, but I'm not a porn star :)
Jeff
Quoted 8 lines · Jeff Garzik
Quoted 8 lines
drivers/ata/libata-scsi.c +8
-0+local_irq_save (flags); + + buflen =ata_scsi_rbuf_get (cmd, &rbuf); + +memset (rbuf,0 , buflen); + rbuf[0 ] = desc ?0xff :0x0 ; + rbuf[1 ] =0x0 ; + rbuf[2 ] =0x2 ;
Quoted 8 lines
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1740
Quoted 6 lines
drivers/ata/libata-scsi.c +6
-0+ rbuf[3 ] =0x0 ; + +ata_scsi_rbuf_put (cmd, rbuf); + +local_irq_restore (flags); +}use
ata_scsi_rbuf_fill()rather than recreating it manually
ata_scsi_rbuf_fill() calls args->done(cmd). Now, I can split the common part of ata_scsi_rbuf_fill into a different function if you'd like, but see below.
Quoted 5 lines
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1757
Quoted 3 lines
drivers/ata/libata-scsi.c +3
-0+/* XXX: fewer than 4 bytes in the page? Doomed. */ + sg->offset +=4 ; + sg->length -=4 ;I'm quite uncomfortable skipping any amount of validation.
Two things ... first, this is just a cheesy hack. I'd like (but I don't see) a generic way to say "I've filled in the first N bytes of this scatterlist, please adjust and continue". That's something that should go in lib/scatterlist.c, IMO.
Second, and more seriously, libata already makes much worse assumptions than this about scatterlists; it's just that the person who wrote them didn't know it (or didn't flag it). Look at the simulation of an INQUIRY, for example. It calls:
ata_scsi_rbuf_fill(&args, ata_scsiop_inq_std);
which does:
buflen = ata_scsi_rbuf_get(cmd, &rbuf); rc = actor(args, rbuf, buflen);
ata_scsi_rbuf_get() does:
buf = kmap_atomic(sg_page(sg), KM_IRQ0) + sg->offset; buflen = sg->length;
ata_scsiop_inq_std() (before it checks buflen) does:
memcpy(rbuf, hdr, sizeof(hdr));
So if a user can persuade the kernel to do an SG transfer directly to userspace and sets up the address one byte before the end of the page, they can persuade libata to write 4 bytes into the page after KM_IRQ0. Which is probably hard to exploit in a meaningful way, but could certainly cause crashes and/or corruption.
Quoted 2 lines
Style in libata is "FIXME" or "TODO" not "XXX". Please help with consistency, it makes grepping easier.
Fine by me.
Quoted 1 line
Plus, I dunno about you, but I'm not a porn star :)
Matthew "Spankalicious" Wilcox.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 17 lines · Matthew Wilcox
Quoted 7 lines · Jeff Garzik
Quoted 7 lines
drivers/ata/libata-scsi.c +7
-0+local_irq_save (flags); + + buflen =ata_scsi_rbuf_get (cmd, &rbuf); + +memset (rbuf,0 , buflen); + rbuf[0 ] = desc ?0xff :0x0 ; + rbuf[1 ] =0x0 ;Quoted 8 lines
drivers/ata/libata-scsi.c void ata_scsi_rbuf_fill(struct ata_scsi_args *args, · line 1740
Quoted 7 lines
drivers/ata/libata-scsi.c +7
-0+ rbuf[2 ] =0x2 ; + rbuf[3 ] =0x0 ; + +ata_scsi_rbuf_put (cmd, rbuf); + +local_irq_restore (flags); +}use
ata_scsi_rbuf_fill()rather than recreating it manually
Quoted 3 lines
ata_scsi_rbuf_fill()callsargs->done(cmd). Now, I can split the common part ofata_scsi_rbuf_fillinto a different function if you'd like, but see below.
No, you're right. My bad: rbuf_fill is for completely emulated commands, not translated commands.
Quoted 4 lines
Second, and more seriously, libata already makes much worse assumptions than this about scatterlists; it's just that the person who wrote them didn't know it (or didn't flag it). Look at the simulation of an INQUIRY, for example. It calls:
[...]
Quoted 5 lines
So if a user can persuade the kernel to do an SG transfer directly to userspace and sets up the address one byte before the end of the page, they can persuade libata to write 4 bytes into the page after
KM_IRQ0. Which is probably hard to exploit in a meaningful way, but could certainly cause crashes and/or corruption.
You arrived at the proper conclusion -- it doesn't happen in practice for a variety of reasons, the main one being that it's largely in the realm of something the superuser must do intentionally, the way kernel and userland code is written today.
Thus, the code intentionally avoids the silly complexity for a case that never truly happens outside of synthetic superuser experiments.
Remember, this and all the other code originated in the days of "if (use_sg)", when scatterlists were not used often for !(read|write) commands.
Quoted 7 lines
Quoted 1 line
Style in libata is "FIXME" or "TODO" not "XXX". Please help with consistency, it makes grepping easier.
Fine by me.
Quoted 1 line
Plus, I dunno about you, but I'm not a porn star :)
Matthew "Spankalicious" Wilcox.
<grin>
Jeff
P.S. I encourage others to investigate making the libata SCSI simulator an optional kernel module.
Long term, a preferred solution would be to use SCSI layer for ATAPI devices, and a kernel block device for ATA devices.
Jeff
Quoted 4 lines
You arrived at the proper conclusion -- it doesn't happen in practice for a variety of reasons, the main one being that it's largely in the realm of something the superuser must do intentionally, the way kernel and userland code is written today.
Actually thats a serious privilege escalation as it gets you from raw disk access to cap_sys_rawio so its a security flaw and should be treated as such even though it is only a minor one in most operating contexts.
Alan
Quoted 4 lines · Jeff Garzik
You arrived at the proper conclusion -- it doesn't happen in practice for a variety of reasons, the main one being that it's largely in the realm of something the superuser must do intentionally, the way kernel and userland code is written today.
I think it could easily happen accidentally. In any case, it's worth fxing.
Quoted 5 lines
Thus, the code intentionally avoids the silly complexity for a case that never truly happens outside of synthetic superuser experiments.
Remember, this and all the other code originated in the days of "if (
use_sg)", when scatterlists were not used often for !(read|write) commands.
I have no desire to increase the complexity of libata, but I think it's possible to fix this in a couple of different ways without adding extra complexity.
One possible way would be for simulate to allocate a page and pass that into the actor functions to fill in. Then simulate could copy that page into the sg lists. This doesn't fix my problem where I need to part-simulate, part-translate, but it's a possible solution for all the various inquiries.
Another would be for the actor functions to set up their own buffers and call something similar to the copy_buffer() function I had as part of the scsi_ram driver I wrote. I happened to write it for the use of INQUIRY too ;-)
+static void copy_buffer(struct scsi_cmnd *cmnd, char *buf, int len) +{
+ char *p; + struct scatterlist *sg; + int i;
+
+ scsi_for_each_sg(cmnd, sg, scsi_sg_count(cmnd), i) {
+ int tocopy = sg->length;
+ if (tocopy > len)
+ tocopy = len;
+
+ p = kmap_atomic(sg_page(sg), KM_USER0); + memcpy(p + sg->offset, buf, tocopy); + kunmap_atomic(p, KM_USER0);
+
+ len -= tocopy;
+ if (!len)
+ break;
+ buf += tocopy;
+ }
+
+ scsi_set_resid(cmnd, len);
+}
Quoted 5 lines
P.S. I encourage others to investigate making the libata SCSI simulator an optional kernel module.Long term, a preferred solution would be to use SCSI layer for ATAPI devices, and a kernel block device for ATA devices.
I think that's the right method to take, though we probably still want to emulate the scsi ioctls.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Jeff Garzik
P.S. I encourage others to investigate making the libata SCSI simulator an optional kernel module.Long term, a preferred solution would be to use SCSI layer for ATAPI devices, and a kernel block device for ATA devices.
I think that's the right method to take, though we probably still want to emulate the scsi ioctls.
We'll want the ATA<->SCSI simulator in its entirety as an optional module for compatibility. People have built applications assuming they must use SCSI (more specifically, SAT) to probe and manipulate ATA devices.
Jeff