From owner-freebsd-bugs@FreeBSD.ORG Tue Mar 23 09:00:56 2004 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9111216A4CE for ; Tue, 23 Mar 2004 09:00:56 -0800 (PST) Received: from mi-exchange.gsilumonics.com (unknown [12.28.116.11]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2762943D49 for ; Tue, 23 Mar 2004 09:00:56 -0800 (PST) (envelope-from WeisgerberJ@gsilumonics.com) Received: by mi-exchange.gsilumonics.com with Internet Mail Service (5.5.2653.19) id ; Tue, 23 Mar 2004 11:54:18 -0500 Message-ID: <09E2AA6AA72BD611AF0E0002B399B684014B41D6@mi-exchange.gsilumonics.com> From: "Weisgerber, John" To: freebsd-bugs@FreeBSD.ORG Date: Tue, 23 Mar 2004 11:54:08 -0500 MIME-Version: 1.0 X-Mailer: Internet Mail Service (5.5.2653.19) Content-Type: text/plain Subject: Possible bug in fwohci driver (#1). X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Mar 2004 17:00:56 -0000 I have been looking at the firewire driver in some detail. I am far from an expert on firewire or device drivers, but I see a couple of things that concern me. One is in the code fragment below taken from fw_ioctl function of fwdev.c. See my comments below the fragment: ==================== case FW_ASYREQ: { struct tcode_info *tinfo; int pay_len = 0; fp = &asyreq->pkt; tinfo = &sc->fc->tcode[fp->mode.hdr.tcode]; if ((tinfo->flag & FWTI_BLOCK_ASY) != 0) pay_len = MAX(0, asyreq->req.len - tinfo->hdr_len); xfer = fw_xfer_alloc_buf(M_FWXFER, pay_len, PAGE_SIZE/*XXX*/); if (xfer == NULL) return (ENOMEM); switch (asyreq->req.type) { case FWASREQNODE: break; case FWASREQEUI: fwdev = fw_noderesolve_eui64(sc->fc, &asyreq->req.dst.eui); if (fwdev == NULL) { device_printf(sc->fc->bdev, "cannot find node\n"); err = EINVAL; goto out; } fp->mode.hdr.dst = FWLOCALBUS | fwdev->dst; break; case FWASRESTL: /* XXX what's this? */ break; case FWASREQSTREAM: /* nothing to do */ break; } bcopy(fp, (void *)&xfer->send.hdr, tinfo->hdr_len); if (pay_len > 0) bcopy((char *)fp + tinfo->hdr_len, (void *)&xfer->send.payload, pay_len); ===================== You will see that the xfer structure is allocated in fw_xfer_alloc_buf() as well as the payload space which is malloc-ed resulting in a kenel virtual address being assigned into the payload structure member. The very last statement does a bcopy into the address of the payload elelment, rather than to the address held in the payload element, which I think was intended. Bottom line is that I suspect the ampersand should be left out of that particular bcopy. The ampersand _does_ belong in the preceeding bcopy, since it is writing the header data to the address of the .hdr element of the structure. It looks to me like if the pay_len is <= 4, then the payload pointer is creamed, but if > 4, then some other data gets creamed as well. I may be missing something here - maybe someone could verify this? I really am not in a position to verify other than theoretically. Best Regards, John Weisgerber GSI Lumonics, Inc. (248) 449-8989 x2638