| Submitter | James Bottomley |
|---|---|
| Date | March 10, 2011, 11:13 p.m. |
| Message ID | <1299798798.11933.167.camel@mulgrave.site> |
| Download | mbox | patch |
| Permalink | /patch/86343/ |
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Comments
On 03/10/2011 06:13 PM, James Bottomley wrote: > I think this stems from a misunderstanding of how the ata error handler > works. ata_scsi_cmd_error_handler() gets called with a passed in list > of commands to handle. However, that list may still not be empty when > it exits. The command ata_scsi_port_error_handler() must be called > (which takes no list) before the list will be completely emptied. This > bites the sas error handler because the two are called from different > functions and the original list has gone out of scope before > ata_scsi_port_error_handler() is called. leading to some commands > dangling on bare stack, which is a potential memory corruption issue. > Fix this by manually deleting all outstanding commands from the on-stack > list before it goes out of scope. Good catch... -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2011-03-10 at 20:28 -0500, Jeff Garzik wrote: > On 03/10/2011 06:13 PM, James Bottomley wrote: > > I think this stems from a misunderstanding of how the ata error handler > > works. ata_scsi_cmd_error_handler() gets called with a passed in list > > of commands to handle. However, that list may still not be empty when > > it exits. The command ata_scsi_port_error_handler() must be called > > (which takes no list) before the list will be completely emptied. This > > bites the sas error handler because the two are called from different > > functions and the original list has gone out of scope before > > ata_scsi_port_error_handler() is called. leading to some commands > > dangling on bare stack, which is a potential memory corruption issue. > > Fix this by manually deleting all outstanding commands from the on-stack > > list before it goes out of scope. > > Good catch... I cannot tell a lie: it was the list debugger code that told me something was wrong ... I just looked at it to see what the problem was. James -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 16c5094..3356bf3 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -802,6 +802,19 @@ int sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q, if (!list_empty(&sata_q)) { ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata cmd error handler\n"); ata_scsi_cmd_error_handler(shost, ap, &sata_q); + /* + * ata's error handler may leave the cmd on the list + * so make sure they don't remain on a stack list + * about to go out of scope. + * + * This looks strange, since the commands are + * now part of no list, but the next error + * action will be ata_port_error_handler() + * which takes no list and sweeps them up + * anyway from the ata tag array. + */ + while (!list_empty(&sata_q)) + list_del_init(sata_q.next); } } while (ap);