SD card read errors are not reported to the host and send bad data

Đang mở
#11,715 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
55/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
c
Lĩnh vực
embedded-iot

Hướng nghiên cứu

Start with tud_msc_read10_cb() and tud_msc_write10_cb() in supervisor/shared/usb/usb_msc_flash.c, checking the result of disk_read() and disk_write() from extmod/vfs_fat_diskio.c. Then read sdcardio_sdcard_readblocks() in shared-module/sdcardio/SDCard.c, where the CMD12 result overwrites the earlier error. Done means failed reads and writes return an error to TinyUSB, the multi-block path returns the first error, and the misleading comment is corrected. Choosing between TUD_MSC_RET_ERROR and TUD_MSC_RET_BUSY needs a maintainer decision.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

bug sdcardio storage usb

Claude wrote this:

In https://github.com/adafruit/circuitpython/issues/11692#issuecomment-6526961492, @jedgarpark found that when sdcardio could not get the SPI bus lock, a failed read was not reported, and sent bad data. On their card, a later write built from those stale sectors emptied a file and wiped a directory.

The comment blamed FatFs, but CircuitPython's own FatFs mount does see the error: disk_read() in extmod/vfs_fat_diskio.c turns any nonzero result into RES_ERROR. The stale data more likely went out over USB, because their build presented the SD card to the computer (CIRCUITPY_SDCARD_USB). Checking main at cf41418682 shows two places where a read error is dropped, and one wrong comment.

Problems
  • tud_msc_read10_cb() in supervisor/shared/usb/usb_msc_flash.c ignores the result of disk_read() and always returns the full byte count. When the read fails, the computer receives whatever was already in the buffer as if it were the card's data. If the computer then writes back a sector built from that data, the filesystem is damaged. tud_msc_write10_cb() likewise ignores the result of disk_write(), so a failed write looks successful to the computer.
  • In sdcardio_sdcard_readblocks() in shared-module/sdcardio/SDCard.c, a multi-block read keeps any error from CMD18 or readinto() in r. It then overwrites r with the result of the CMD12 stop command, so a failed multi-block read can return 0.
  • The comment // No caller actually uses this value. at the end of sdcardio_sdcard_readblocks() is wrong. disk_read() uses the value as described above, and SDCard.readblocks() raises OSError on a negative value.
Possible fix
  • Return TUD_MSC_RET_ERROR from tud_msc_read10_cb() and tud_msc_write10_cb() when disk_read() or disk_write() does not return RES_OK. TinyUSB then fails the SCSI command instead of sending the buffer. Returning TUD_MSC_RET_BUSY instead would make TinyUSB retry the callback, which might suit a bus-lock timeout better.
  • In the multi-block path, save the first error before sending CMD12 and return it.
  • Remove or correct the comment.
Ngôn ngữ chính
C
Star
4.6k
Fork
1.4k
Merge trung bình
1 ngày 13 giờ
Pull request đã merge (30 ngày)
171

Chuẩn bị môi trường

Dự án này không cung cấp dev container, Dockerfile hay hướng dẫn đóng góp, nên bạn cần tự thiết lập môi trường: hãy bắt đầu từ README và xem hướng dẫn đóng góp lần đầu của chúng tôi để biết các bước chung.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của adafruit/circuitpython

Tất cả issue của adafruit/circuitpython

Issue tương tự

Thêm issue về C

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.