| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20120807221951.037825200@linuxfoundation.org>
Date: Tue, 7 Aug 2012 15:25:19 -0700
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc: Greg KH <gregkh@...uxfoundation.org>,
torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
alan@...rguk.ukuu.org.uk, Gerd Hoffmann <kraxel@...hat.com>
Subject: [ 031/122] Revert "usb/uas: make sure data urb is gone if we receive status before that"
From: Greg KH <gregkh@...uxfoundation.org>
3.5-stable review patch. If anyone has any objections, please let me know.
------------------
From: Gerd Hoffmann <kraxel@...hat.com>
commit c621a81edecdee85da32c566c21836332c764fda upstream.
This reverts commit e4d8318a85779b25b880187b1b1c44e797bd7d4b.
This patch makes uas.c call usb_unlink_urb on data urbs. The data urbs
get freed in the completion callback. This is illegal according to the
usb_unlink_urb documentation.
This patch also makes the code expect the data completion callback
being called before the status completion callback. This isn't
guaranteed to be the case, even though the actual data transfer should
be finished by the time the status is received.
Background: The ehci irq handler for example only know that there are
finished transfers, it then has go check the QHs & TDs to see which
transfers did actually finish. It has no way to figure in which order
the transfers did complete. The xhci driver can call the callbacks in
completion order thanks to the event queue. This does nicely explain
why the driver is solid on a (usb2) xhci port whereas it goes crazy on
ehci in my testing.
Signed-off-by: Gerd Hoffmann <kraxel@...hat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
drivers/usb/storage/uas.c | 90 +++++++---------------------------------------
1 file changed, 15 insertions(+), 75 deletions(-)
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -58,9 +58,6 @@ enum {
SUBMIT_DATA_OUT_URB = (1 << 5),
ALLOC_CMD_URB = (1 << 6),
SUBMIT_CMD_URB = (1 << 7),
- COMPLETED_DATA_IN = (1 << 8),
- COMPLETED_DATA_OUT = (1 << 9),
- DATA_COMPLETES_CMD = (1 << 10),
};
/* Overrides scsi_pointer */
@@ -114,7 +111,6 @@ static void uas_sense(struct urb *urb, s
{
struct sense_iu *sense_iu = urb->transfer_buffer;
struct scsi_device *sdev = cmnd->device;
- struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
if (urb->actual_length > 16) {
unsigned len = be16_to_cpup(&sense_iu->len);
@@ -132,15 +128,13 @@ static void uas_sense(struct urb *urb, s
}
cmnd->result = sense_iu->status;
- if (!(cmdinfo->state & DATA_COMPLETES_CMD))
- cmnd->scsi_done(cmnd);
+ cmnd->scsi_done(cmnd);
}
static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu_old *sense_iu = urb->transfer_buffer;
struct scsi_device *sdev = cmnd->device;
- struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
if (urb->actual_length > 8) {
unsigned len = be16_to_cpup(&sense_iu->len) - 2;
@@ -158,8 +152,7 @@ static void uas_sense_old(struct urb *ur
}
cmnd->result = sense_iu->status;
- if (!(cmdinfo->state & DATA_COMPLETES_CMD))
- cmnd->scsi_done(cmnd);
+ cmnd->scsi_done(cmnd);
}
static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
@@ -184,7 +177,6 @@ static void uas_stat_cmplt(struct urb *u
struct Scsi_Host *shost = urb->context;
struct uas_dev_info *devinfo = (void *)shost->hostdata[0];
struct scsi_cmnd *cmnd;
- struct uas_cmd_info *cmdinfo;
u16 tag;
int ret;
@@ -210,32 +202,12 @@ static void uas_stat_cmplt(struct urb *u
dev_err(&urb->dev->dev, "failed submit status urb\n");
return;
}
- cmdinfo = (void *)&cmnd->SCp;
switch (iu->iu_id) {
case IU_ID_STATUS:
if (devinfo->cmnd == cmnd)
devinfo->cmnd = NULL;
- if (!(cmdinfo->state & COMPLETED_DATA_IN) &&
- cmdinfo->data_in_urb) {
- if (devinfo->use_streams) {
- cmdinfo->state |= DATA_COMPLETES_CMD;
- usb_unlink_urb(cmdinfo->data_in_urb);
- } else {
- usb_free_urb(cmdinfo->data_in_urb);
- }
- }
- if (!(cmdinfo->state & COMPLETED_DATA_OUT) &&
- cmdinfo->data_out_urb) {
- if (devinfo->use_streams) {
- cmdinfo->state |= DATA_COMPLETES_CMD;
- usb_unlink_urb(cmdinfo->data_in_urb);
- } else {
- usb_free_urb(cmdinfo->data_out_urb);
- }
- }
-
if (urb->actual_length < 16)
devinfo->uas_sense_old = 1;
if (devinfo->uas_sense_old)
@@ -264,59 +236,27 @@ static void uas_stat_cmplt(struct urb *u
dev_err(&urb->dev->dev, "failed submit status urb\n");
}
-static void uas_data_out_cmplt(struct urb *urb)
-{
- struct scsi_cmnd *cmnd = urb->context;
- struct scsi_data_buffer *sdb = scsi_out(cmnd);
- struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
-
- cmdinfo->state |= COMPLETED_DATA_OUT;
-
- sdb->resid = sdb->length - urb->actual_length;
- usb_free_urb(urb);
-
- if (cmdinfo->state & DATA_COMPLETES_CMD)
- cmnd->scsi_done(cmnd);
-}
-
-static void uas_data_in_cmplt(struct urb *urb)
+static void uas_data_cmplt(struct urb *urb)
{
- struct scsi_cmnd *cmnd = urb->context;
- struct scsi_data_buffer *sdb = scsi_in(cmnd);
- struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
-
- cmdinfo->state |= COMPLETED_DATA_IN;
-
+ struct scsi_data_buffer *sdb = urb->context;
sdb->resid = sdb->length - urb->actual_length;
usb_free_urb(urb);
-
- if (cmdinfo->state & DATA_COMPLETES_CMD)
- cmnd->scsi_done(cmnd);
}
static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
- unsigned int pipe, struct scsi_cmnd *cmnd,
- enum dma_data_direction dir)
+ unsigned int pipe, u16 stream_id,
+ struct scsi_data_buffer *sdb,
+ enum dma_data_direction dir)
{
- struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct usb_device *udev = devinfo->udev;
struct urb *urb = usb_alloc_urb(0, gfp);
- struct scsi_data_buffer *sdb;
- usb_complete_t complete_fn;
- u16 stream_id = cmdinfo->stream;
if (!urb)
goto out;
- if (dir == DMA_FROM_DEVICE) {
- sdb = scsi_in(cmnd);
- complete_fn = uas_data_in_cmplt;
- } else {
- sdb = scsi_out(cmnd);
- complete_fn = uas_data_out_cmplt;
- }
- usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length,
- complete_fn, cmnd);
- urb->stream_id = stream_id;
+ usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt,
+ sdb);
+ if (devinfo->use_streams)
+ urb->stream_id = stream_id;
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
urb->sg = sdb->table.sgl;
out:
@@ -418,8 +358,8 @@ static int uas_submit_urbs(struct scsi_c
if (cmdinfo->state & ALLOC_DATA_IN_URB) {
cmdinfo->data_in_urb = uas_alloc_data_urb(devinfo, gfp,
- devinfo->data_in_pipe, cmnd,
- DMA_FROM_DEVICE);
+ devinfo->data_in_pipe, cmdinfo->stream,
+ scsi_in(cmnd), DMA_FROM_DEVICE);
if (!cmdinfo->data_in_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_IN_URB;
@@ -436,8 +376,8 @@ static int uas_submit_urbs(struct scsi_c
if (cmdinfo->state & ALLOC_DATA_OUT_URB) {
cmdinfo->data_out_urb = uas_alloc_data_urb(devinfo, gfp,
- devinfo->data_out_pipe, cmnd,
- DMA_TO_DEVICE);
+ devinfo->data_out_pipe, cmdinfo->stream,
+ scsi_out(cmnd), DMA_TO_DEVICE);
if (!cmdinfo->data_out_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_OUT_URB;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists