Skip to content

Commit e4d8318

Browse files
Sebastian Andrzej SiewiorSarah Sharp
Sebastian Andrzej Siewior
authored and
Sarah Sharp
committed
usb/uas: make sure data urb is gone if we receive status before that
Just run into the following: - new disk arrived in the system - udev couldn't wait to get its hands on to to run ata_id /dev/sda - this sent the cdb 0xa1 to the device. - my UAS-gadget recevied the cdb and had no idea what to do with it. It decided to send a status URB back with sense set to invalid opcode. - the host side received it status and completed the scsi command. - the host sent another scsi with 4kib data buffer - Now I was confused why the data transfer is only 512 bytes instead of 4kib since the host is always allocating the complete transfer in one go. - Finally the system crashed while walking through the sg list. This patch adds three new flags in order to distinguish between DATA URB completed and outstanding. If we receive status before data, we cancel data and let data complete the command. This solves the problem for IN and OUT transfers but does not work for BIDI. Signed-off-by: Sebastian Andrzej Siewior <[email protected]> Signed-off-by: Sarah Sharp <[email protected]>
1 parent ee398b5 commit e4d8318

File tree

1 file changed

+75
-15
lines changed

1 file changed

+75
-15
lines changed

‎drivers/usb/storage/uas.c

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ enum {
5858
SUBMIT_DATA_OUT_URB = (1 << 5),
5959
ALLOC_CMD_URB = (1 << 6),
6060
SUBMIT_CMD_URB = (1 << 7),
61+
COMPLETED_DATA_IN = (1 << 8),
62+
COMPLETED_DATA_OUT = (1 << 9),
63+
DATA_COMPLETES_CMD = (1 << 10),
6164
};
6265

6366
/* Overrides scsi_pointer */
@@ -111,6 +114,7 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
111114
{
112115
struct sense_iu *sense_iu = urb->transfer_buffer;
113116
struct scsi_device *sdev = cmnd->device;
117+
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
114118

115119
if (urb->actual_length > 16) {
116120
unsigned len = be16_to_cpup(&sense_iu->len);
@@ -128,13 +132,15 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
128132
}
129133

130134
cmnd->result = sense_iu->status;
131-
cmnd->scsi_done(cmnd);
135+
if (!(cmdinfo->state & DATA_COMPLETES_CMD))
136+
cmnd->scsi_done(cmnd);
132137
}
133138

134139
static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
135140
{
136141
struct sense_iu_old *sense_iu = urb->transfer_buffer;
137142
struct scsi_device *sdev = cmnd->device;
143+
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
138144

139145
if (urb->actual_length > 8) {
140146
unsigned len = be16_to_cpup(&sense_iu->len) - 2;
@@ -152,7 +158,8 @@ static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
152158
}
153159

154160
cmnd->result = sense_iu->status;
155-
cmnd->scsi_done(cmnd);
161+
if (!(cmdinfo->state & DATA_COMPLETES_CMD))
162+
cmnd->scsi_done(cmnd);
156163
}
157164

158165
static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
@@ -177,6 +184,7 @@ static void uas_stat_cmplt(struct urb *urb)
177184
struct Scsi_Host *shost = urb->context;
178185
struct uas_dev_info *devinfo = (void *)shost->hostdata[0];
179186
struct scsi_cmnd *cmnd;
187+
struct uas_cmd_info *cmdinfo;
180188
u16 tag;
181189
int ret;
182190

@@ -202,12 +210,32 @@ static void uas_stat_cmplt(struct urb *urb)
202210
dev_err(&urb->dev->dev, "failed submit status urb\n");
203211
return;
204212
}
213+
cmdinfo = (void *)&cmnd->SCp;
205214

206215
switch (iu->iu_id) {
207216
case IU_ID_STATUS:
208217
if (devinfo->cmnd == cmnd)
209218
devinfo->cmnd = NULL;
210219

220+
if (!(cmdinfo->state & COMPLETED_DATA_IN) &&
221+
cmdinfo->data_in_urb) {
222+
if (devinfo->use_streams) {
223+
cmdinfo->state |= DATA_COMPLETES_CMD;
224+
usb_unlink_urb(cmdinfo->data_in_urb);
225+
} else {
226+
usb_free_urb(cmdinfo->data_in_urb);
227+
}
228+
}
229+
if (!(cmdinfo->state & COMPLETED_DATA_OUT) &&
230+
cmdinfo->data_out_urb) {
231+
if (devinfo->use_streams) {
232+
cmdinfo->state |= DATA_COMPLETES_CMD;
233+
usb_unlink_urb(cmdinfo->data_in_urb);
234+
} else {
235+
usb_free_urb(cmdinfo->data_out_urb);
236+
}
237+
}
238+
211239
if (urb->actual_length < 16)
212240
devinfo->uas_sense_old = 1;
213241
if (devinfo->uas_sense_old)
@@ -236,27 +264,59 @@ static void uas_stat_cmplt(struct urb *urb)
236264
dev_err(&urb->dev->dev, "failed submit status urb\n");
237265
}
238266

239-
static void uas_data_cmplt(struct urb *urb)
267+
static void uas_data_out_cmplt(struct urb *urb)
268+
{
269+
struct scsi_cmnd *cmnd = urb->context;
270+
struct scsi_data_buffer *sdb = scsi_out(cmnd);
271+
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
272+
273+
cmdinfo->state |= COMPLETED_DATA_OUT;
274+
275+
sdb->resid = sdb->length - urb->actual_length;
276+
usb_free_urb(urb);
277+
278+
if (cmdinfo->state & DATA_COMPLETES_CMD)
279+
cmnd->scsi_done(cmnd);
280+
}
281+
282+
static void uas_data_in_cmplt(struct urb *urb)
240283
{
241-
struct scsi_data_buffer *sdb = urb->context;
284+
struct scsi_cmnd *cmnd = urb->context;
285+
struct scsi_data_buffer *sdb = scsi_in(cmnd);
286+
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
287+
288+
cmdinfo->state |= COMPLETED_DATA_IN;
289+
242290
sdb->resid = sdb->length - urb->actual_length;
243291
usb_free_urb(urb);
292+
293+
if (cmdinfo->state & DATA_COMPLETES_CMD)
294+
cmnd->scsi_done(cmnd);
244295
}
245296

246297
static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
247-
unsigned int pipe, u16 stream_id,
248-
struct scsi_data_buffer *sdb,
249-
enum dma_data_direction dir)
298+
unsigned int pipe, struct scsi_cmnd *cmnd,
299+
enum dma_data_direction dir)
250300
{
301+
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
251302
struct usb_device *udev = devinfo->udev;
252303
struct urb *urb = usb_alloc_urb(0, gfp);
304+
struct scsi_data_buffer *sdb;
305+
usb_complete_t complete_fn;
306+
u16 stream_id = cmdinfo->stream;
253307

254308
if (!urb)
255309
goto out;
256-
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt,
257-
sdb);
258-
if (devinfo->use_streams)
259-
urb->stream_id = stream_id;
310+
if (dir == DMA_FROM_DEVICE) {
311+
sdb = scsi_in(cmnd);
312+
complete_fn = uas_data_in_cmplt;
313+
} else {
314+
sdb = scsi_out(cmnd);
315+
complete_fn = uas_data_out_cmplt;
316+
}
317+
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length,
318+
complete_fn, cmnd);
319+
urb->stream_id = stream_id;
260320
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
261321
urb->sg = sdb->table.sgl;
262322
out:
@@ -358,8 +418,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
358418

359419
if (cmdinfo->state & ALLOC_DATA_IN_URB) {
360420
cmdinfo->data_in_urb = uas_alloc_data_urb(devinfo, gfp,
361-
devinfo->data_in_pipe, cmdinfo->stream,
362-
scsi_in(cmnd), DMA_FROM_DEVICE);
421+
devinfo->data_in_pipe, cmnd,
422+
DMA_FROM_DEVICE);
363423
if (!cmdinfo->data_in_urb)
364424
return SCSI_MLQUEUE_DEVICE_BUSY;
365425
cmdinfo->state &= ~ALLOC_DATA_IN_URB;
@@ -376,8 +436,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
376436

377437
if (cmdinfo->state & ALLOC_DATA_OUT_URB) {
378438
cmdinfo->data_out_urb = uas_alloc_data_urb(devinfo, gfp,
379-
devinfo->data_out_pipe, cmdinfo->stream,
380-
scsi_out(cmnd), DMA_TO_DEVICE);
439+
devinfo->data_out_pipe, cmnd,
440+
DMA_TO_DEVICE);
381441
if (!cmdinfo->data_out_urb)
382442
return SCSI_MLQUEUE_DEVICE_BUSY;
383443
cmdinfo->state &= ~ALLOC_DATA_OUT_URB;

0 commit comments

Comments
 (0)