misc/64623: Possible memory corruption in fwohci driver - fwdev.c

John Weisgerber weisgerberj at gsilumonics.com
Tue Mar 23 09:10:17 PST 2004


>Number:         64623
>Category:       misc
>Synopsis:       Possible memory corruption in fwohci driver - fwdev.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Mar 23 09:10:17 PST 2004
>Closed-Date:
>Last-Modified:
>Originator:     John Weisgerber
>Release:        Current CVS tree.
>Organization:
GSI Lumonics
>Environment:
I was reviewing the driver code.
>Description:
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);
=====================
Hopefully that wasn't too garbled by the web interface...


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 element, 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

      
>How-To-Repeat:
      
>Fix:
See full description - remove the offending ampersand(?)
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list