From nobody Sat Nov 6 16:33:08 2021 X-Original-To: scsi@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id A04381847DC9; Sat, 6 Nov 2021 16:33:11 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Hmjbq4BHcz3Crk; Sat, 6 Nov 2021 16:33:11 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from [192.168.0.88] (unknown [195.64.148.76]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: avg/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id CCEE123BEF; Sat, 6 Nov 2021 16:33:10 +0000 (UTC) (envelope-from avg@FreeBSD.org) To: scsi@freebsd.org, hackers@FreeBSD.org From: Andriy Gapon Subject: cam_periph_(un)mapmem issues with XPT_MMC_IO Message-ID: <62ee1f87-6240-708b-b324-49c87e0efca8@FreeBSD.org> Date: Sat, 6 Nov 2021 18:33:08 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.14.0 List-Id: SCSI subsystem List-Archive: https://lists.freebsd.org/archives/freebsd-scsi List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-scsi@freebsd.org X-BeenThere: freebsd-scsi@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-ThisMailContainsUnwantedMimeParts: N 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