USB: gadget: storage: remove alignment assumption

This patch (as1481) fixes a problem affecting g_file_storage and
g_mass_storage when running at SuperSpeed.  The two drivers currently
assume that the bulk-out maxpacket size can evenly divide the SCSI
block size, which is 512 bytes.  But SuperSpeed bulk endpoints have a
maxpacket size of 1024, so the assumption is no longer true.

This patch removes that assumption from the drivers, by getting rid of
a small optimization (they try to align VFS reads and writes on page
cache boundaries).  If a command's starting logical block address is
512 bytes below the end of a page, it's not okay to issue a USB
command for just those 512 bytes when the maxpacket size is 1024 -- it
would result in either babble (for an OUT transfer) or a short packet
(for an IN transfer).

Also, for backward compatibility, the test for writes extending beyond
the end of the backing storage has to be changed.  If the host tries
to do this, we should accept the data that fits in the backing storage
and ignore the rest.  Because the storage's end may not align with a
USB packet boundary, this means we may have to accept a USB OUT
transfer that extends beyond the end of the storage and then write out
only the part of the data that fits.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 1cdb1fa..4306a83 100644 (file)
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -744,7 +744,6 @@ static int do_read(struct fsg_common *common)
        u32                     amount_left;
        loff_t                  file_offset, file_offset_tmp;
        unsigned int            amount;
-       unsigned int            partial_page;
        ssize_t                 nread;
 
        /*
@@ -783,18 +782,10 @@ static int do_read(struct fsg_common *common)
                 * Try to read the remaining amount.
                 * But don't read more than the buffer size.
                 * And don't try to read past the end of the file.
-                * Finally, if we're not at a page boundary, don't read past
-                *      the next page.
-                * If this means reading 0 then we were asked to read past
-                *      the end of file.
                 */
                amount = min(amount_left, FSG_BUFLEN);
                amount = min((loff_t)amount,
                             curlun->file_length - file_offset);
-               partial_page = file_offset & (PAGE_CACHE_SIZE - 1);
-               if (partial_page > 0)
-                       amount = min(amount, (unsigned int)PAGE_CACHE_SIZE -
-                                            partial_page);
 
                /* Wait for the next buffer to become available */
                bh = common->next_buffhd_to_fill;
@@ -840,6 +831,12 @@ static int do_read(struct fsg_common *common)
                file_offset  += nread;
                amount_left  -= nread;
                common->residue -= nread;
+
+               /*
+                * Except at the end of the transfer, nread will be
+                * equal to the buffer size, which is divisible by the
+                * bulk-in maxpacket size.
+                */
                bh->inreq->length = nread;
                bh->state = BUF_STATE_FULL;
 
@@ -878,7 +875,6 @@ static int do_write(struct fsg_common *common)
        u32                     amount_left_to_req, amount_left_to_write;
        loff_t                  usb_offset, file_offset, file_offset_tmp;
        unsigned int            amount;
-       unsigned int            partial_page;
        ssize_t                 nwritten;
        int                     rc;
 
@@ -934,24 +930,13 @@ static int do_write(struct fsg_common *common)
 
                        /*
                         * Figure out how much we want to get:
-                        * Try to get the remaining amount.
-                        * But don't get more than the buffer size.
-                        * And don't try to go past the end of the file.
-                        * If we're not at a page boundary,
-                        *      don't go past the next page.
-                        * If this means getting 0, then we were asked
-                        *      to write past the end of file.
-                        * Finally, round down to a block boundary.
+                        * Try to get the remaining amount,
+                        * but not more than the buffer size.
                         */
                        amount = min(amount_left_to_req, FSG_BUFLEN);
-                       amount = min((loff_t)amount,
-                                    curlun->file_length - usb_offset);
-                       partial_page = usb_offset & (PAGE_CACHE_SIZE - 1);
-                       if (partial_page > 0)
-                               amount = min(amount,
-       (unsigned int)PAGE_CACHE_SIZE - partial_page);
-
-                       if (amount == 0) {
+
+                       /* Beyond the end of the backing file? */
+                       if (usb_offset >= curlun->file_length) {
                                get_some_more = 0;
                                curlun->sense_data =
                                        SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
@@ -960,16 +945,6 @@ static int do_write(struct fsg_common *common)
                                curlun->info_valid = 1;
                                continue;
                        }
-                       amount = round_down(amount, curlun->blksize);
-                       if (amount == 0) {
-
-                               /*
-                                * Why were we were asked to transfer a
-                                * partial block?
-                                */
-                               get_some_more = 0;
-                               continue;
-                       }
 
                        /* Get the next buffer */
                        usb_offset += amount;
@@ -979,8 +954,9 @@ static int do_write(struct fsg_common *common)
                                get_some_more = 0;
 
                        /*
-                        * amount is always divisible by 512, hence by
-                        * the bulk-out maxpacket size
+                        * Except at the end of the transfer, amount will be
+                        * equal to the buffer size, which is divisible by
+                        * the bulk-out maxpacket size.
                         */
                        bh->outreq->length = amount;
                        bh->bulk_out_intended_length = amount;
@@ -1019,6 +995,11 @@ static int do_write(struct fsg_common *common)
                                amount = curlun->file_length - file_offset;
                        }
 
+                       /* Don't write a partial block */
+                       amount = round_down(amount, curlun->blksize);
+                       if (amount == 0)
+                               goto empty_write;
+
                        /* Perform the write */
                        file_offset_tmp = file_offset;
                        nwritten = vfs_write(curlun->filp,
@@ -1051,6 +1032,7 @@ static int do_write(struct fsg_common *common)
                                break;
                        }
 
+ empty_write:
                        /* Did the host decide to stop early? */
                        if (bh->outreq->actual != bh->outreq->length) {
                                common->short_packet_received = 1;
@@ -1151,8 +1133,6 @@ static int do_verify(struct fsg_common *common)
                 * Try to read the remaining amount, but not more than
                 * the buffer size.
                 * And don't try to read past the end of the file.
-                * If this means reading 0 then we were asked to read
-                * past the end of file.
                 */
                amount = min(amount_left, FSG_BUFLEN);
                amount = min((loff_t)amount,
@@ -1628,7 +1608,8 @@ static int throw_away_data(struct fsg_common *common)
                        amount = min(common->usb_amount_left, FSG_BUFLEN);
 
                        /*
-                        * amount is always divisible by 512, hence by
+                        * Except at the end of the transfer, amount will be
+                        * equal to the buffer size, which is divisible by
                         * the bulk-out maxpacket size.
                         */
                        bh->outreq->length = amount;

diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index 59d9775..c6f96a2 100644 (file)
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -1135,7 +1135,6 @@ static int do_read(struct fsg_dev *fsg)
        u32                     amount_left;
        loff_t                  file_offset, file_offset_tmp;
        unsigned int            amount;
-       unsigned int            partial_page;
        ssize_t                 nread;
 
        /* Get the starting Logical Block Address and check that it's
@@ -1170,17 +1169,10 @@ static int do_read(struct fsg_dev *fsg)
                 * Try to read the remaining amount.
                 * But don't read more than the buffer size.
                 * And don't try to read past the end of the file.
-                * Finally, if we're not at a page boundary, don't read past
-                *      the next page.
-                * If this means reading 0 then we were asked to read past
-                *      the end of file. */
+                */
                amount = min((unsigned int) amount_left, mod_data.buflen);
                amount = min((loff_t) amount,
                                curlun->file_length - file_offset);
-               partial_page = file_offset & (PAGE_CACHE_SIZE - 1);
-               if (partial_page > 0)
-                       amount = min(amount, (unsigned int) PAGE_CACHE_SIZE -
-                                       partial_page);
 
                /* Wait for the next buffer to become available */
                bh = fsg->next_buffhd_to_fill;
@@ -1225,6 +1217,11 @@ static int do_read(struct fsg_dev *fsg)
                file_offset  += nread;
                amount_left  -= nread;
                fsg->residue -= nread;
+
+               /* Except at the end of the transfer, nread will be
+                * equal to the buffer size, which is divisible by the
+                * bulk-in maxpacket size.
+                */
                bh->inreq->length = nread;
                bh->state = BUF_STATE_FULL;
 
@@ -1261,7 +1258,6 @@ static int do_write(struct fsg_dev *fsg)
        u32                     amount_left_to_req, amount_left_to_write;
        loff_t                  usb_offset, file_offset, file_offset_tmp;
        unsigned int            amount;
-       unsigned int            partial_page;
        ssize_t                 nwritten;
        int                     rc;
 
@@ -1312,23 +1308,13 @@ static int do_write(struct fsg_dev *fsg)
                if (bh->state == BUF_STATE_EMPTY && get_some_more) {
 
                        /* Figure out how much we want to get:
-                        * Try to get the remaining amount.
-                        * But don't get more than the buffer size.
-                        * And don't try to go past the end of the file.
-                        * If we're not at a page boundary,
-                        *      don't go past the next page.
-                        * If this means getting 0, then we were asked
-                        *      to write past the end of file.
-                        * Finally, round down to a block boundary. */
+                        * Try to get the remaining amount,
+                        * but not more than the buffer size.
+                        */
                        amount = min(amount_left_to_req, mod_data.buflen);
-                       amount = min((loff_t) amount, curlun->file_length -
-                                       usb_offset);
-                       partial_page = usb_offset & (PAGE_CACHE_SIZE - 1);
-                       if (partial_page > 0)
-                               amount = min(amount,
-       (unsigned int) PAGE_CACHE_SIZE - partial_page);
-
-                       if (amount == 0) {
+
+                       /* Beyond the end of the backing file? */
+                       if (usb_offset >= curlun->file_length) {
                                get_some_more = 0;
                                curlun->sense_data =
                                        SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
@@ -1336,14 +1322,6 @@ static int do_write(struct fsg_dev *fsg)
                                curlun->info_valid = 1;
                                continue;
                        }
-                       amount = round_down(amount, curlun->blksize);
-                       if (amount == 0) {
-
-                               /* Why were we were asked to transfer a
-                                * partial block? */
-                               get_some_more = 0;
-                               continue;
-                       }
 
                        /* Get the next buffer */
                        usb_offset += amount;
@@ -1352,8 +1330,10 @@ static int do_write(struct fsg_dev *fsg)
                        if (amount_left_to_req == 0)
                                get_some_more = 0;
 
-                       /* amount is always divisible by 512, hence by
-                        * the bulk-out maxpacket size */
+                       /* Except at the end of the transfer, amount will be
+                        * equal to the buffer size, which is divisible by
+                        * the bulk-out maxpacket size.
+                        */
                        bh->outreq->length = bh->bulk_out_intended_length =
                                        amount;
                        bh->outreq->short_not_ok = 1;
@@ -1389,6 +1369,11 @@ static int do_write(struct fsg_dev *fsg)
                                amount = curlun->file_length - file_offset;
                        }
 
+                       /* Don't write a partial block */
+                       amount = round_down(amount, curlun->blksize);
+                       if (amount == 0)
+                               goto empty_write;
+
                        /* Perform the write */
                        file_offset_tmp = file_offset;
                        nwritten = vfs_write(curlun->filp,
@@ -1421,6 +1406,7 @@ static int do_write(struct fsg_dev *fsg)
                                break;
                        }
 
+ empty_write:
                        /* Did the host decide to stop early? */
                        if (bh->outreq->actual != bh->outreq->length) {
                                fsg->short_packet_received = 1;
@@ -1517,8 +1503,7 @@ static int do_verify(struct fsg_dev *fsg)
                 * Try to read the remaining amount, but not more than
                 * the buffer size.
                 * And don't try to read past the end of the file.
-                * If this means reading 0 then we were asked to read
-                * past the end of file. */
+                */
                amount = min((unsigned int) amount_left, mod_data.buflen);
                amount = min((loff_t) amount,
                                curlun->file_length - file_offset);
@@ -1981,8 +1966,10 @@ static int throw_away_data(struct fsg_dev *fsg)
                        amount = min(fsg->usb_amount_left,
                                        (u32) mod_data.buflen);
 
-                       /* amount is always divisible by 512, hence by
-                        * the bulk-out maxpacket size */
+                       /* Except at the end of the transfer, amount will be
+                        * equal to the buffer size, which is divisible by
+                        * the bulk-out maxpacket size.
+                        */
                        bh->outreq->length = bh->bulk_out_intended_length =
                                        amount;
                        bh->outreq->short_not_ok = 1;