diff mbox series

usb: gadget: mass_storage: Fix cdrom data transfers on MAC-OS

Message ID 1661535142-5204-1-git-send-email-quic_kriskura@quicinc.com (mailing list archive)
State Superseded
Headers show
Series usb: gadget: mass_storage: Fix cdrom data transfers on MAC-OS | expand

Commit Message

Krishna Kurapati Aug. 26, 2022, 5:32 p.m. UTC
During cdrom emulation, the response to read_toc command must contain
the cdrom address as the number of sectors (2048 byte sized blocks)
represented either as an absolute value (when MSF bit is '0') or in
terms of PMin/PSec/PFrame (when MSF bit is set to '1'). Incase of
cdrom, the fsg_lun_open call sets the number of sectors to 2048 byte
sized blocks.

When MAC OS sends a read_toc request with MSF set to '1', the
store_cdrom_address assumes that the address being provided is the
LUN size represented in 512 byte sized blocks instead of 2048. It
tries to modify the address further to convert it to 2048 byte sized
blocks and store it in MSF format. This results in data transfer
failures as the cdrom address being provided in the read_toc response
is incorrect.

Fixes: 3f565a363cee ("usb: gadget: storage: adapt logic block size to bound block devices")
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
 drivers/usb/gadget/function/storage_common.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Alan Stern Aug. 26, 2022, 6:54 p.m. UTC | #1
On Fri, Aug 26, 2022 at 11:02:22PM +0530, Krishna Kurapati wrote:
> During cdrom emulation, the response to read_toc command must contain
> the cdrom address as the number of sectors (2048 byte sized blocks)
> represented either as an absolute value (when MSF bit is '0') or in
> terms of PMin/PSec/PFrame (when MSF bit is set to '1'). Incase of
> cdrom, the fsg_lun_open call sets the number of sectors to 2048 byte

Sets the sector size to 2048 bytes, not the number of sectors.

> sized blocks.
> 
> When MAC OS sends a read_toc request with MSF set to '1', the
> store_cdrom_address assumes that the address being provided is the
> LUN size represented in 512 byte sized blocks instead of 2048. It
> tries to modify the address further to convert it to 2048 byte sized
> blocks and store it in MSF format. This results in data transfer
> failures as the cdrom address being provided in the read_toc response
> is incorrect.
> 
> Fixes: 3f565a363cee ("usb: gadget: storage: adapt logic block size to bound block devices")
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>

Should include "Cc: stable@vger.kernel.org".

> ---
>  drivers/usb/gadget/function/storage_common.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/storage_common.c b/drivers/usb/gadget/function/storage_common.c
> index 03035db..db40392 100644
> --- a/drivers/usb/gadget/function/storage_common.c
> +++ b/drivers/usb/gadget/function/storage_common.c
> @@ -295,7 +295,6 @@ void store_cdrom_address(u8 *dest, int msf, u32 addr)
>  {
>  	if (msf) {
>  		/* Convert to Minutes-Seconds-Frames */
> -		addr >>= 2;		/* Convert to 2048-byte frames */

Please leave a comment here saying that we already know the sector size 
is 2048 bytes.

>  		addr += 2*75;		/* Lead-in occupies 2 seconds */
>  		dest[3] = addr % 75;	/* Frames */
>  		addr /= 75;

Otherwise this is okay.  In your next submission you can add:

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/storage_common.c b/drivers/usb/gadget/function/storage_common.c
index 03035db..db40392 100644
--- a/drivers/usb/gadget/function/storage_common.c
+++ b/drivers/usb/gadget/function/storage_common.c
@@ -295,7 +295,6 @@  void store_cdrom_address(u8 *dest, int msf, u32 addr)
 {
 	if (msf) {
 		/* Convert to Minutes-Seconds-Frames */
-		addr >>= 2;		/* Convert to 2048-byte frames */
 		addr += 2*75;		/* Lead-in occupies 2 seconds */
 		dest[3] = addr % 75;	/* Frames */
 		addr /= 75;