Messages in this thread | | | Date | Tue, 09 Apr 2024 07:34:41 +0200 | From | "Arnd Bergmann" <> | Subject | Re: [PATCH 1/3] [v2] staging: rts5208: replace weird strncpy() with memcpy() |
| |
On Mon, Apr 8, 2024, at 22:28, Justin Stitt wrote: > On Mon, Apr 08, 2024 at 09:48:09PM +0200, Arnd Bergmann wrote:
>> @@ -523,7 +523,7 @@ static int inquiry(struct scsi_cmnd *srb, struct rtsx_chip *chip) >> >> if (sendbytes > 8) { >> memcpy(buf, inquiry_buf, 8); >> - strncpy(buf + 8, inquiry_string, sendbytes - 8); >> + memcpy(buf + 8, inquiry_string, min(sendbytes, 36) - 8); > > I must say I am not the biggest fan of manual string management with raw > pointer offsets. I wonder if scnprintf() could achieve your goal here of > combining inquiry_buf with inquiry_string into buf (perhaps utilizing > %.*s format specifier).
scnprintf() would be wrong here, since it's not actually a string but a SCSI HW data structure with both binary and ASCII members and no NUL termination. It's supposed to be padded with spaces.
> With that being said, I am just a casual reader of this code and I > really don't know much about the expected behavior of `buf` > (NUL-terminated, NUL-padded, etc) or even what the next line buf[4]=0x33 > does.
I believe this sets the length of the buffer to 51 bytes when pro_formatter_flag is set, overriding the 0x1f (31 bytes) for the normal length.
The root of the problem here is that the driver emulates a SCSI command set on a card reader for both SD cards and memory sticks instead of using the existing abstractions from drivers/{mmc,memstick}.
Apparently drivers/misc/cardreader/rtsx_pcr.c does this the right way for all later devices (rts5209 and up) but is not compatible with this variant of the hardware.
Arnd
| |