cam_periph_(un)mapmem issues with XPT_MMC_IO
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 06 Nov 2021 16:33:08 UTC
I think that I see a few issues with cam_periph_(un)mapmem handling of XPT_MMC_IO ccbs. First, I think that this piece of code sets the length incorrectly: data_ptrs[0] = (unsigned char **)&ccb->mmcio.cmd.data; lengths[0] = sizeof(struct mmc_data *); I think that it should be sizeof(struct mmc_data) as it seems that we want to map the whole structure into the kernel memory. Then, I think that this code is not safe / correct: data_ptrs[1] = (unsigned char **)&ccb->mmcio.cmd.data->data; lengths[1] = ccb->mmcio.cmd.data->len; I believe that the code accesses internals of ccb->mmcio.cmd.data via a userland pointer as that object is not mapped into the kernel address space yet and the pointer is not adjusted yet. Finally, I think that there is a problem with unmapping of those two data buffers. In all other cases where cam_periph_unmapmem() works on multiple memory locations (numbufs > 2), those locations are independent of each other. But for XPT_MMC_IO one buffer contains a pointer to other buffer, so there is a dependency between them. It seems that there is an access to mmcio.cmd.data object via its kernel pointer after the object is unmapped from the kernel space because it comes before mmcio.cmd.data->data in the array. Running a command like # camcontrol mmcsdcmd 2:0:0 -c 8 -a 0 -f 0x35 -l 512 I get the following crash (on arm): panic: vm_fault_lookup: fault on nofault entry, addr: 0xde367000 cpuid = 2 time = 1636189704 KDB: stack backtrace: db_trace_self() at db_trace_self db_trace_self_wrapper() at db_trace_self_wrapper+0x30 vpanic() at vpanic+0x17c doadump() at doadump vm_fault() at vm_fault+0x17b0 vm_fault_trap() at vm_fault_trap+0x78 abort_handler() at abort_handler+0x3c8 exception_exit() at exception_exit cam_periph_unmapmem() at cam_periph_unmapmem+0x2e4 passsendccb() at passsendccb+0x194 passdoioctl() at passdoioctl+0xcc passioctl() at passioctl+0x28 devfs_ioctl() at devfs_ioctl+0xcc vn_ioctl() at vn_ioctl+0x12c devfs_ioctl_f() at devfs_ioctl_f+0x2c kern_ioctl() at kern_ioctl+0x318 sys_ioctl() at sys_ioctl+0x108 Unfortunately I do not have a crash dump, only a serial console output. As far as I can tell cam_periph_unmapmem+0x2e4 corresponds to the assignment in the following code: /* Set the user's pointer back to the original value */ *data_ptrs[i] = mapinfo->orig[i]; As a quick hack I tried to reverse the order of iteration and it seems to have fixed the crash. In general, it seems that cam_periph_(un)mapmem is not good for the MMC case because of the dependency between buffers. Perhaps the code could be extended to handle dependent buffers. Or maybe MMC should get its own special routines for mapping and unmapping the buffers. Warner and Alexander, I Bcc-ed you for r320844 / a94a63f0a6bc1 and r345656 / b059686a71c89. One added XPT_MMC_IO to cam_periph_mapmem and the other added XPT_MMC_IO to cam_periph_unmapmem along with multitude of other changes. -- Andriy Gapon