| 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
| ||
|
Date: Tue, 22 Mar 2011 11:13:50 -0400 (EDT)
From: Alan Stern <stern@...land.harvard.edu>
To: Roger Quadros <roger.quadros@...ia.com>
cc: gregkh@...e.de, <sshtylyov@...sta.com>,
<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] usb: gadget: file_storage: Make CD-ROM emulation
work with Mac OS-X
On Tue, 22 Mar 2011, Roger Quadros wrote:
> However, if host asked for data more than the TOC length then residue will be
> greater than zero and this will result in zero padded transfers.
>
> Not a big issue but should we be fixing it?
>
> one way could be to set fsg->residue to actual TOC length. in do_read_toc().
The current behavior is required by the the Bulk-Only Transport
specification. The spec says (section 6.7.2, case (4) or (5)):
If the device intends to send less data than the host indicated, then:
The device shall send the intended data.
The device may send fill data to pad up to a total of
dCBWDataTransferLength.
If the device actually transfers less data than the host indicated,
then:
The device may end the transfer with a short packet.
The device shall STALL the Bulk-In pipe.
You're loading the mass-storage gadget with the "stall=n" module
parameter, right? Therefore the driver is not allowed to halt the
Bulk-In endpoint, and therefore the driver is not allowed to send less
data than the host indicated, which means the data has to be padded.
On the other hand, I don't think any implementations would get upset if
we simply ended the transfer with a short packet instead of adhering
strictly to the spec.
The patch below should do what you want. I haven't tested it.
Alan Stern
drivers/usb/gadget/f_mass_storage.c | 54 +++++++-----------------------------
drivers/usb/gadget/file_storage.c | 53 ++++++++---------------------------
2 files changed, 23 insertions(+), 84 deletions(-)
Index: usb-2.6/drivers/usb/gadget/f_mass_storage.c
===================================================================
--- usb-2.6.orig/drivers/usb/gadget/f_mass_storage.c
+++ usb-2.6/drivers/usb/gadget/f_mass_storage.c
@@ -1584,37 +1584,6 @@ static int wedge_bulk_in_endpoint(struct
return rc;
}
-static int pad_with_zeros(struct fsg_dev *fsg)
-{
- struct fsg_buffhd *bh = fsg->common->next_buffhd_to_fill;
- u32 nkeep = bh->inreq->length;
- u32 nsend;
- int rc;
-
- bh->state = BUF_STATE_EMPTY; /* For the first iteration */
- fsg->common->usb_amount_left = nkeep + fsg->common->residue;
- while (fsg->common->usb_amount_left > 0) {
-
- /* Wait for the next buffer to be free */
- while (bh->state != BUF_STATE_EMPTY) {
- rc = sleep_thread(fsg->common);
- if (rc)
- return rc;
- }
-
- nsend = min(fsg->common->usb_amount_left, FSG_BUFLEN);
- memset(bh->buf + nkeep, 0, nsend - nkeep);
- bh->inreq->length = nsend;
- bh->inreq->zero = 0;
- start_transfer(fsg, fsg->bulk_in, bh->inreq,
- &bh->inreq_busy, &bh->state);
- bh = fsg->common->next_buffhd_to_fill = bh->next;
- fsg->common->usb_amount_left -= nsend;
- nkeep = 0;
- }
- return 0;
-}
-
static int throw_away_data(struct fsg_common *common)
{
struct fsg_buffhd *bh;
@@ -1702,6 +1671,10 @@ static int finish_reply(struct fsg_commo
if (common->data_size == 0) {
/* Nothing to send */
+ /* Don't know what to do if common->fsg is NULL */
+ } else if (!common->fsg) {
+ rc = -EIO;
+
/* If there's no residue, simply send the last buffer */
} else if (common->residue == 0) {
bh->inreq->zero = 0;
@@ -1710,24 +1683,19 @@ static int finish_reply(struct fsg_commo
common->next_buffhd_to_fill = bh->next;
/*
- * For Bulk-only, if we're allowed to stall then send the
- * short packet and halt the bulk-in endpoint. If we can't
- * stall, pad out the remaining data with 0's.
+ * For Bulk-only, mark the end of the data with a short
+ * packet. If we are allowed to stall, halt the bulk-in
+ * endpoint. (Note: This violates the Bulk-Only Transport
+ * specification, which requires us to pad the data if we
+ * don't halt the endpoint. Presumably nobody will mind.)
*/
- } else if (common->can_stall) {
+ } else {
bh->inreq->zero = 1;
if (!start_in_transfer(common, bh))
- /* Don't know what to do if
- * common->fsg is NULL */
rc = -EIO;
common->next_buffhd_to_fill = bh->next;
- if (common->fsg)
+ if (common->can_stall)
rc = halt_bulk_in_endpoint(common->fsg);
- } else if (fsg_is_set(common)) {
- rc = pad_with_zeros(common->fsg);
- } else {
- /* Don't know what to do if common->fsg is NULL */
- rc = -EIO;
}
break;
Index: usb-2.6/drivers/usb/gadget/file_storage.c
===================================================================
--- usb-2.6.orig/drivers/usb/gadget/file_storage.c
+++ usb-2.6/drivers/usb/gadget/file_storage.c
@@ -1947,37 +1947,6 @@ static int wedge_bulk_in_endpoint(struct
return rc;
}
-static int pad_with_zeros(struct fsg_dev *fsg)
-{
- struct fsg_buffhd *bh = fsg->next_buffhd_to_fill;
- u32 nkeep = bh->inreq->length;
- u32 nsend;
- int rc;
-
- bh->state = BUF_STATE_EMPTY; // For the first iteration
- fsg->usb_amount_left = nkeep + fsg->residue;
- while (fsg->usb_amount_left > 0) {
-
- /* Wait for the next buffer to be free */
- while (bh->state != BUF_STATE_EMPTY) {
- rc = sleep_thread(fsg);
- if (rc)
- return rc;
- }
-
- nsend = min(fsg->usb_amount_left, (u32) mod_data.buflen);
- memset(bh->buf + nkeep, 0, nsend - nkeep);
- bh->inreq->length = nsend;
- bh->inreq->zero = 0;
- start_transfer(fsg, fsg->bulk_in, bh->inreq,
- &bh->inreq_busy, &bh->state);
- bh = fsg->next_buffhd_to_fill = bh->next;
- fsg->usb_amount_left -= nsend;
- nkeep = 0;
- }
- return 0;
-}
-
static int throw_away_data(struct fsg_dev *fsg)
{
struct fsg_buffhd *bh;
@@ -2082,18 +2051,20 @@ static int finish_reply(struct fsg_dev *
}
}
- /* For Bulk-only, if we're allowed to stall then send the
- * short packet and halt the bulk-in endpoint. If we can't
- * stall, pad out the remaining data with 0's. */
+ /*
+ * For Bulk-only, mark the end of the data with a short
+ * packet. If we are allowed to stall, halt the bulk-in
+ * endpoint. (Note: This violates the Bulk-Only Transport
+ * specification, which requires us to pad the data if we
+ * don't halt the endpoint. Presumably nobody will mind.)
+ */
else {
- if (mod_data.can_stall) {
- bh->inreq->zero = 1;
- start_transfer(fsg, fsg->bulk_in, bh->inreq,
- &bh->inreq_busy, &bh->state);
- fsg->next_buffhd_to_fill = bh->next;
+ bh->inreq->zero = 1;
+ start_transfer(fsg, fsg->bulk_in, bh->inreq,
+ &bh->inreq_busy, &bh->state);
+ fsg->next_buffhd_to_fill = bh->next;
+ if (mod_data.can_stall)
rc = halt_bulk_in_endpoint(fsg);
- } else
- rc = pad_with_zeros(fsg);
}
break;
--
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