COKPOWEHEU / usb

MIT License
27 stars 13 forks source link

Несколько странных мест в коде для Mass Storage #2

Open Dmitro25 opened 2 years ago

Dmitro25 commented 2 years ago

Здравствуйте! Я прочитал Вашу статью на Хабре и хотел воспользоваться Вашим кодом (мне только нужно его модифицировать для другого микроконтроллера). Столкнулся с рядом странных мест в файле usb_class_msd.c:

  1. функция scsi_mode_sense_6(). Зачем производится присваивание buffer[4] = 0; ... buffer[8] = 0; передаваться же будет только 4 байта?
  2. функция scsi_write_10(). Присваивание msc_sense.key = CSW_STATUS_FAILED; выглядит как-то подозрительно - там, вроде, справа должна быть константа из перечисления enum sbc_sense_key.
  3. функция scsi_mmc_read_fmt_cap(). Выражение
    buffer[3] = //size
    buffer[4] = (last_lba >> 24) & 0xFF;

    наверное, стоит переписать как

    buffer[3] = 8; //size
    buffer[4] = (last_lba >> 24) & 0xFF;

    иначе получается, что в buffer[3] оказывается старшая часть номера последнего блока, а не размер последующей структуры.

  4. Там же. Согласно описанию команды SCSI_MMC_READ_FORMAT_CAPACITY, в ней нужно передавать не номер последнего блока (как в команде SCSI_READ_CAPACITY), а число блоков, т.е. из выражения uint32_t last_lba = storage[lun].capacity / 512 - 1; нужно убрать "-1".
COKPOWEHEU commented 2 years ago
  1. функция scsi_mode_sense_6(). Зачем производится присваивание buffer[4] = 0; ... buffer[8] = 0; передаваться же будет только 4 байта?

Должно быть, это осталось со времен отладки. Думаю, заполнение лишних полей можно убрать. Кстати, надеюсь, вы разобрались с buffer[2], что для lun0 он устанавливает readonly флаг, а для lun1 - readwrite.

  1. функция scsi_write_10(). Присваивание msc_sense.key = CSW_STATUS_FAILED; выглядит как-то подозрительно - там, вроде, справа должна быть константа из перечисления enum sbc_sense_key.

Там идея в том, что при попытке записать в readonly память должна вернуться ошибка, а потом, если хост запросит подробности по scsi_request_sense, ему вернутся подробности "SBC_ASC_WRITE_PROTECTED". Насколько я понял из документации, key получает только статус, без подробностей.

  1. buffer[3] = //size

Ох блин, как же я это проморгал... а я еще гадаю, чего это винда больше двух lun'ов видеть отказывается... По-хорошему, там надо 8*(maxlun+1), и соответствующие поля тоже заполнить. Ваш вариант подойдет для единственного lun'а, это не универсальное решение.

  1. Там же. Согласно описанию команды SCSI_MMC_READ_FORMAT_CAPACITY, в ней нужно передавать не номер последнего блока (как в команде SCSI_READ_CAPACITY), а число блоков, т.е. из выражения uint32_t last_lba = storage[lun].capacity / 512 - 1; нужно убрать "-1".

Согласен. Исправлю.

Здравствуйте! Я прочитал Вашу статью на Хабре и хотел воспользоваться Вашим кодом (мне только нужно его модифицировать для другого микроконтроллера).

Когда портируете, отпишитесь пожалуйста. Наверняка ведь найдется что у вас стащить. . Ну и разумеется, большое спасибо за обратную связь, да и вообще внимательность. Уж дурацкую ошибку с закомментированным size надо исправлить как можно скорее... пока еще кто-нибудь не заметил...

Dmitro25 commented 2 years ago

Там идея в том, что при попытке записать в readonly память должна вернуться ошибка, а потом, если хост запросит подробности по scsi_request_sense, ему вернутся подробности "SBC_ASC_WRITE_PROTECTED". Насколько я понял из документации, key получает только статус, без подробностей.

Это я по контексту понял. Просто константа CSW_STATUS_FAILED, которая у Вас используется, она предназначена для заполнения поля bStatus структуры usb_msc_csw. А Вы её записываете в поле usb_msc_sense.key, которое должно принимать значение типа sbc_sense_key. Получается, что Вы, по сути, записываете в usb_msc_sense.key значение SBC_SENSE_KEY_RECOVERED_ERROR (равное 0x01). Не уверен, что это правильно.

Ну и разумеется, большое спасибо за обратную связь, да и вообще внимательность.

Да не за что. Это Вам спасибо, что проделали такую большую работу и поделились результатами с сообществом.

COKPOWEHEU commented 2 years ago

Вроде замеченное исправил. Теперь все 16 "флешек" отображаются даже в некоторых реальных виндах (надо еще 10-ю и 11-ю где-то найти для тестов). Заодно поправил пару других багов, переставил обработчики прерываний (надеюсь, будет быстрее работать) и добавил дефолтный колбэк для конечных точек. Проверяйте ничего ли я не забыл.