On Fri, Aug 26, 2022 at 01:05:06AM +0530, Krishna Kurapati PSSNV wrote:
> Hi Alan, Maxim,
>
> Sorry to multicast a mail.
Messages like this should always be CC'ed to the linux-usb mailing list.
Even if they are purely speculative.
> I am seeing one regression in emulation of my
> linux target on MAC-OS. I configured my usb_gadget containing mass_storage
> composition as follows:
>
> 1) echo "yes" > mass_storage.0/lun.0/cdrom
> 2) echo "yes" > mass_storage.0/lun.0/removable
> 3) echo "yes" > mass_storage.0/lun.0/nofua
> 4) echo "/sdcard/download/autorun.iso" > mass_storage.0/lun.0/file
>
> When I try to emulate it on Windows Host, it mounts fine and I am able to
> copy all the files from my iso file to Windows PC. But when I try to do the
> same with Mac-OS, I see that although it emulates, I am not able to copy all
> the files from my iso to Macbook. At some point after data copy has begun, I
> encounter the following error:
> "The Finder can't complete the operatin because some data in file can't be
> read or written. (Error code -36)"
>
> I suspected if we are not providing/advertising the contents of cdrom
> properly and wanted to check the diff between Windows/Mac and turns out if I
> make the following changes, it works fine :
>
> --- 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;
>
>
> Mac-OS has MSF bit set to '1' in its read_toc request and Windows sets it to
> '0' in its read_toc request. In case of Mac-OS, the cdrom addr which
> represents the curlun->num_sectors is already in blocks of size 2048 bytes
> as configured by fsg_lun_open(..) with
>
> if (curlun->cdrom) {
> blksize = 2048;
> blkbits = 11;
>
> num_sectors = size >> blkbits; /* File size in logic-block-size blocks */
>
> I did a little digging and found out the following patch [1]
>
> In this patch, the fsg_lun_open call had the following code:
> num_sectors = size >> 9; /* File size in 512-byte blocks */
>
> And so, we had to shift it by 2 more positions right in store_cdrom_address
> to make the number of sectors as 2048 byte blocks, but the latest code seems
> to be doing it already and shifting it further by 2 bits is affecting
> emulation on MAC.
>
> Can you help understand if the cdrom address is really messed up or my
> analysis that response to read_toc cmd is messed up when MSF is set to '1'
> is wrong ?
>
> Wanted to confirm with you guys before pushing a patch as the pinged patch
> [1] has been done 11 years ago and its highly unlikely that it is buggy.
> Sorry again for unicasting a mail.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3f565a363cee14d2ed281823196d455bfc7c4170
You're right; the code in store_cdrom_address() is buggy. Apparently
the 3f565a363cee commit overlooked this one use of the block size. The
fact that this hasn't been noticed for 11 years probably just means that
the this part of the code hasn't been used much in that time.
Please submit a patch to fix the bug.
Alan Stern
