Bug 277115 - sysutils/cdrdao: zero length and odd-sized DMA transfer attempted
Summary: sysutils/cdrdao: zero length and odd-sized DMA transfer attempted
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Jason E. Hale
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-17 17:29 UTC by Benjamin Jacobs
Modified: 2024-02-20 00:53 UTC (History)
0 users

See Also:
jhale: maintainer-feedback+


Attachments
0001-sysutils-cdrdao-Zero-or-odd-length-DMA-fix.patch (2.61 KB, patch)
2024-02-17 17:31 UTC, Benjamin Jacobs
no flags Details | Diff
0001-sysutils-cdrdao-Zero-or-odd-length-DMA-fix.patch (9.28 KB, patch)
2024-02-17 18:23 UTC, Benjamin Jacobs
no flags Details | Diff
patch-dao_ScsiIf-freebsd-cam.cc (364 bytes, patch)
2024-02-18 14:23 UTC, Benjamin Jacobs
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Jacobs 2024-02-17 17:29:38 UTC
Cdrdao doesn't work on (S)ATA drives anymore:

cdrdao disk-info --device /dev/cd0
/dev/cd0:  	Rev:
Using driver: Generic SCSI-3/MMC - Version 2.0 (options 0x0000)

ERROR: Cannot setup device /dev/cd0.

dmesg shows:
ata0: FAILURE - zero length DMA transfer attempted
ata0: setting up DMA failed

This is caused by a bad SCSI CCB having CAM_DIR_IN and a zero length (presumably FreeBSD started to reject zero-sized DMA SCSI transfer), fixed by using CAM_DIR_NONE in such case.

After this issue is fixed, for any cd with an odd number of tracks there will be the following error (note that read-cd and "slow mode" read-toc will still work):

cdrdao read-toc --device /dev/cd0 --fast-toc -v9999   toc-file
...
Reading toc data...
getTocGeneric: data len 124
Raw toc data len: 191
ERROR: Cannot read disk toc.

And dmesg to show:

ata0: FAILURE - odd-sized DMA transfer attempt 191 % 2
ata0: setting up DMA failed

191 is 11 bytes * 17 tracks + 4, read from the disc, see dao/GenericMMC.cc:GenericMMC::getRawToc  


The attached patch adds a cdrdao patch that fixes both issues. It also
sets the GNU_CONFIGURE_MANPREFIX according to the new rule, and portrevision is incremented.

After patch:
cdrdao read-toc --device /dev/cd0 --fast-toc -v9999   toc-file
/dev/cd0:  	Rev:
Reading driver table from file "/usr/local/share/cdrdao/drivers".
Found 316 valid driver table entries.
Using driver: Generic SCSI-3/MMC - Version 2.0 (options 0x0000)

Reading toc data...
getTocGeneric: data len 124
Raw toc data len: 191
Raw toc contains HEX values.
[output truncated]
Comment 1 Benjamin Jacobs 2024-02-17 17:31:21 UTC
Created attachment 248537 [details]
0001-sysutils-cdrdao-Zero-or-odd-length-DMA-fix.patch
Comment 2 Benjamin Jacobs 2024-02-17 18:23:50 UTC
Created attachment 248539 [details]
0001-sysutils-cdrdao-Zero-or-odd-length-DMA-fix.patch

Revised patch to avoid overflowing the read buffer.
Comment 3 Jason E. Hale freebsd_committer freebsd_triage 2024-02-17 19:06:05 UTC
Strange. I just tested several CDs with odd and even track numbers and was unable to reproduce this. cdrdao works as intended for me on FreeBSD 15-CURRENT (amd64) with a Matshita Blu-ray laptop drive . I haven’t tried your patch yet.
Comment 4 Benjamin Jacobs 2024-02-18 14:15:49 UTC
That the problem may be inexisting with the ahci(4) driver? My system has an ICH7 controller, it looks like it is using the ata(4) driver.

atapci0: <Intel ICH7 UDMA100 controller> port 0x1f0-0x1f7,0x3f6,0x170-0x177,0x376,0xffa0-0xffaf at device 31.1 on pci0
ata0: <ATA channel> at channel 0 on atapci0
atapci1: <Intel ICH7 SATA300 controller> port 0xb800-0xb807,0xb400-0xb403,0xb000-0xb007,0xa800-0xa803,0xa400-0xa40f irq 17 at device 31.2 on pci0
ata2: <ATA channel> at channel 0 on atapci1
ata3: <ATA channel> at channel 1 on atapci1
ada0 at ata2 bus 0 scbus1 target 0 lun 0
cd3 at ata3 bus 0 scbus2 target 1 lun 0
cd2 at ata2 bus 0 scbus1 target 1 lun 0
cd1 at ata0 bus 0 scbus0 target 1 lun 0
cd0 at ata0 bus 0 scbus0 target 0 lun 0

The errors comes from sys/dev/ata/ata-dma.c, around lines 277-287 in ata_dmaload().

   277      if (!request->bytecount) {
   278          device_printf(request->parent,
   279                        "FAILURE - zero length DMA transfer attempted\n");
   280          return EIO;
   281      }
   282      if (request->bytecount & (ch->dma.alignment - 1)) {
   283          device_printf(request->parent,
   284                        "FAILURE - odd-sized DMA transfer attempt %d %% %d\n",
   285                        request->bytecount, ch->dma.alignment);
   286          return EIO;
   287      }

The alignment check has been relaxed for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=127316 , requiring only the size to be a multiple of the alignement. It isn't clear to me how userspace can detect it. It may be always 2 (ata/ata-dma.c:ata_dmainit() ) or maybe 16 on some chipset chipsets/ata-{cyrix,national}.c?

The CAM_DIR_NONE par of the fix seems unlikely to cause any problem on other systems. It is sufficient for my usage of cdrdao. I'll attach the minimal patch that I've been using before cooking a workaround for the odd-sized length.
Comment 5 Benjamin Jacobs 2024-02-18 14:23:26 UTC
Created attachment 248560 [details]
patch-dao_ScsiIf-freebsd-cam.cc

This patches only fixes the zero length DMA error.
Comment 6 Benjamin Jacobs 2024-02-18 15:15:43 UTC
Investigating a bit more, the patch-dao_ScsiIf-freebsd-cam.cc does not make much sense because flags is already CAM_DIR_NONE as per initializer, and the data_len, data_prt should already be zeroed by the bzero() call... so maybe some compiler optimization kicks in (maybe it doesn't detect the bzero and assumes data_len/ptr are left uninitialized?), and that messes with the macro cam_fill_csio?
The following patch also fixes the issue... it is strange.


--- dao/ScsiIf-freebsd-cam.cc.orig      2024-02-18 14:48:05 UTC
+++ dao/ScsiIf-freebsd-cam.cc
@@ -128,6 +128,7 @@ int ScsiIf::sendCmd(const unsigned char *cmd, int cmdL
                data_len = dataInLen;
                flags = CAM_DIR_IN;
        }
+       fprintf(stderr, "dataOut %p, dataOutLen %d, dataIn %p, dataInLen %d, flags %p\n", dataOut, dataOutLen, dataIn, dataInLen, flags);

        cam_fill_csio(&impl_->ccb->csio,
                      DEF_RETRY_COUNT,
Comment 7 Jason E. Hale freebsd_committer freebsd_triage 2024-02-19 20:04:37 UTC
> That the problem may be inexisting with the ahci(4) driver? My system has an ICH7 controller, it looks like it is using the ata(4) driver.

You're quite right. My system is using the ahci(4) driver. The oldest working hardware I have lying around is a laptop from 2007 with an ICH8M southbridge which supports AHCI, so all I can really do is check for regressions.

Besides the manpage fixes, and thank you very much for taking that into consideration, is the patch you put in comment #6 all that is needed to fix this for you then or do need some time to cook?
Comment 8 Benjamin Jacobs 2024-02-19 23:41:02 UTC
> Besides the manpage fixes, and thank you very much for taking that into consideration, is the patch you put in comment #6 all that is needed to fix this for you then or do need some time to cook?

You are welcome, thank you for looking into this report. I came up with a better patch that works for me, so please use the patch inlined here:

patch-dao_ScsiIf-freebsd-cam.cc
--- dao/ScsiIf-freebsd-cam.cc.orig	2023-01-25 14:30:35 UTC
+++ dao/ScsiIf-freebsd-cam.cc
@@ -112,8 +112,8 @@ int ScsiIf::sendCmd(const unsigned char *cmd, int cmdL
 {
 	int		retval;
 	int		flags = CAM_DIR_NONE;
-	u_int8_t *	data_ptr;
-	size_t		data_len;
+	u_int8_t *	data_ptr = NULL;
+	size_t		data_len = 0;
 
 	bzero(impl_->ccb, sizeof(union ccb));
 	bcopy(cmd, &impl_->ccb->csio.cdb_io.cdb_bytes, cmdLen);


I think that underlying issue was caused by the compiler optimizing away the call to cam_fill_csio because data_ptr and data_len were left unitialized in the case of neither dataIn nor dataOut, and since cam_fill_csio didn't copy the flags=CAM_DIR_NONE into the CCB, its was implitly left to CAM_DIR_BOTH = 0x00000000 and that would trigger the issue in dev/ata/ata_dma.c.

The issue about odd-sized dma might still occur on ata(4), but that's a minor annoyance; read-cd and read-toc in the default slow scan mode work and that's enought for my use-case.
Comment 9 Jason E. Hale freebsd_committer freebsd_triage 2024-02-20 00:53:23 UTC
Excellent! We should probably also convert legacy bzero(3) calls to memset(3) and bcopy(3) to memmove(3), respectively, and upstream. Upstream cdrdao [1] seams rather unresponsive these days, however, but optical media is dying out, so it's to be expected. I'm quite happy to host the changes locally in the meantime, however.

[1] https://github.com/cdrdao/cdrdao/