From admin@citylink.dinoex.sub.org Mon Nov 28 23:43:57 2011 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A9C2C1065745 for ; Mon, 28 Nov 2011 23:43:57 +0000 (UTC) (envelope-from admin@citylink.dinoex.sub.org) Received: from uucp.dinoex.sub.de (uucp.dinoex.sub.de [194.45.71.2]) by mx1.freebsd.org (Postfix) with ESMTP id 427CA8FC19 for ; Mon, 28 Nov 2011 23:43:57 +0000 (UTC) Received: from uucp.dinoex.sub.de (uucp@uucp.dinoex.sub.de [194.45.71.2] (may be forged)) by uucp.dinoex.sub.de (8.14.4/8.14.4) with ESMTP id pASND4Sq069219 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 29 Nov 2011 00:13:05 +0100 (CET) (envelope-from admin@citylink.dinoex.sub.org) Received: from citylink.dinoex.sub.org (uucp@localhost) by uucp.dinoex.sub.de (8.14.4/8.14.4/Submit) with UUCP id pASND4nR069218 for FreeBSD-gnats-submit@freebsd.org; Tue, 29 Nov 2011 00:13:04 +0100 (CET) (envelope-from admin@citylink.dinoex.sub.org) Received: from gate.oper.dinoex.org (gate-e [192.168.98.2]) by citylink.dinoex.sub.de (8.14.5/8.14.5) with ESMTP id pASMLHvl008325 for ; Mon, 28 Nov 2011 23:21:17 +0100 (CET) (envelope-from admin@edge.oper.dinoex.org) Received: from edge.oper.dinoex.org (gate-e [192.168.98.2]) by gate.oper.dinoex.org (8.14.5/8.14.5) with ESMTP id pASMKJta008246 for ; Mon, 28 Nov 2011 23:20:19 +0100 (CET) (envelope-from admin@edge.oper.dinoex.org) Received: from edge.oper.dinoex.org (edge-e1.oper.dinoex.org [192.168.97.9]) by edge.oper.dinoex.org (8.14.5/8.14.5) with ESMTP id pASMJ0wo008093 for ; Mon, 28 Nov 2011 23:19:01 +0100 (CET) (envelope-from admin@edge.oper.dinoex.org) Received: (from admin@localhost) by edge.oper.dinoex.org (8.14.5/8.14.5/Submit) id pASMJ0G4008092; Mon, 28 Nov 2011 23:19:00 +0100 (CET) (envelope-from admin) Message-Id: <201111282219.pASMJ0G4008092@edge.oper.dinoex.org> Date: Mon, 28 Nov 2011 23:19:00 +0100 (CET) From: Peter Much Reply-To: Peter Much To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: [ed][panic][patch] large traffic yields occasional panics X-Send-Pr-Version: 3.113 X-GNATS-Notify: >Number: 162932 >Category: kern >Synopsis: [ed][panic][patch] large traffic yields occasional panics >Confidential: no >Severity: serious >Priority: medium >Responsible: yongari >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Nov 28 23:50:12 UTC 2011 >Closed-Date: Fri Jan 06 00:53:44 UTC 2012 >Last-Modified: Fri Jan 06 00:53:44 UTC 2012 >Originator: Peter Much >Release: FreeBSD 7.4-STABLE i386 >Organization: n/a >Environment: System: FreeBSD edge.oper.dinoex.org 7.4-STABLE FreeBSD 7.4-STABLE #1: Sun Nov 27 04:10:51 UTC 2011 root@disp.oper.dinoex.org:/usr/src/sys/i386/compile/E1R74V1 i386 Interface card is WD8013 (ISA 16bit). I found (and fixed) this with RELEASE-7.2, but it seems I forgot to publish it. The patch does still apply cleanly, so the bug may still linger around, or it may not. Also, it may be applicaple to higher releases too, or it may not. >Description: Panic happens when sending large amounts of data. From my understanding, that seems to happen when an mbuf content has uneven bytesize and ends at a page boundary. >How-To-Repeat: 1. have such nice old hardware, and 2. just send data over it. >Fix: *** sys/dev/ed/if_ed.c.orig Sat Nov 12 02:31:09 2011 --- sys/dev/ed/if_ed.c Sat Nov 12 02:56:03 2011 *************** *** 1690,1700 **** } } for (len = 0; m != 0; m = m->m_next) { ! if (sc->isa16bit) ! bus_space_write_region_2(sc->mem_bst, ! sc->mem_bsh, dst, ! mtod(m, uint16_t *), (m->m_len + 1)/ 2); ! else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); --- 1690,1704 ---- } } for (len = 0; m != 0; m = m->m_next) { ! if (sc->isa16bit) { ! if(m->m_len > 1) ! bus_space_write_region_2(sc->mem_bst, ! sc->mem_bsh, dst, mtod(m, uint16_t *), ! m->m_len / 2); ! bus_space_write_1(sc->mem_bst, sc->mem_bsh, ! dst + m->m_len - 1, ! *(mtod(m, uint8_t *) + m->m_len - 1)); ! } else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); >Release-Note: >Audit-Trail: Responsible-Changed-From-To: freebsd-bugs->freebsd-net Responsible-Changed-By: linimon Responsible-Changed-When: Tue Nov 29 04:41:49 UTC 2011 Responsible-Changed-Why: Over to maintainer(s). http://www.freebsd.org/cgi/query-pr.cgi?pr=162932 State-Changed-From-To: open->feedback State-Changed-By: yongari State-Changed-When: Tue Nov 29 23:01:36 UTC 2011 State-Changed-Why: It seems that the code path referenced invalid address when a mbuf length is odd bytes. I created a new diff based on your patch since your patch may still invoke bus_space_write_1(9) if mbuf length is even bytes. I also added mbuf length check and used NULL in comparing a mbuf pointer. You can find the patch at the following URL. http://people.freebsd.org/~yongari/ed.pf.diff Let me know whether this patch works for you. Responsible-Changed-From-To: freebsd-net->yongari Responsible-Changed-By: yongari Responsible-Changed-When: Tue Nov 29 23:01:36 UTC 2011 Responsible-Changed-Why: Grab. http://www.freebsd.org/cgi/query-pr.cgi?pr=162932 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/162932: commit references a PR Date: Mon, 5 Dec 2011 18:10:52 +0000 (UTC) Author: yongari Date: Mon Dec 5 18:10:43 2011 New Revision: 228286 URL: http://svn.freebsd.org/changeset/base/228286 Log: Fix off by one error in mbuf access. Previously it caused panic. While I'm here use NULL to compare mbuf pointer and add additional check for zero length mbuf before accessing the mbuf. PR: kern/162932 Modified: head/sys/dev/ed/if_ed.c Modified: head/sys/dev/ed/if_ed.c ============================================================================== --- head/sys/dev/ed/if_ed.c Mon Dec 5 17:44:12 2011 (r228285) +++ head/sys/dev/ed/if_ed.c Mon Dec 5 18:10:43 2011 (r228286) @@ -1709,12 +1709,19 @@ ed_shmem_write_mbufs(struct ed_softc *sc break; } } - for (len = 0; m != 0; m = m->m_next) { - if (sc->isa16bit) - bus_space_write_region_2(sc->mem_bst, - sc->mem_bsh, dst, - mtod(m, uint16_t *), (m->m_len + 1)/ 2); - else + for (len = 0; m != NULL; m = m->m_next) { + if (m->m_len == 0) + continue; + if (sc->isa16bit) { + if (m->m_len > 1) + bus_space_write_region_2(sc->mem_bst, + sc->mem_bsh, dst, + mtod(m, uint16_t *), m->m_len / 2); + if ((m->m_len & 1) != 0) + bus_space_write_1(sc->mem_bst, sc->mem_bsh, + dst + m->m_len - 1, + *(mtod(m, uint8_t *) + m->m_len - 1)); + } else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: feedback->patched State-Changed-By: yongari State-Changed-When: Mon Dec 5 18:23:33 UTC 2011 State-Changed-Why: Fixed in HEAD(r228286). Thanks for your reporting and testing! http://www.freebsd.org/cgi/query-pr.cgi?pr=162932 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/162932: commit references a PR Date: Thu, 5 Jan 2012 20:50:07 +0000 (UTC) Author: yongari Date: Thu Jan 5 20:49:48 2012 New Revision: 229648 URL: http://svn.freebsd.org/changeset/base/229648 Log: MFC r228286: Fix off by one error in mbuf access. Previously it caused panic. While I'm here use NULL to compare mbuf pointer and add additional check for zero length mbuf before accessing the mbuf. PR: kern/162932 Modified: stable/9/sys/dev/ed/if_ed.c Directory Properties: stable/9/sys/ (props changed) stable/9/sys/amd64/include/xen/ (props changed) stable/9/sys/boot/ (props changed) stable/9/sys/boot/i386/efi/ (props changed) stable/9/sys/boot/ia64/efi/ (props changed) stable/9/sys/boot/ia64/ski/ (props changed) stable/9/sys/boot/powerpc/boot1.chrp/ (props changed) stable/9/sys/boot/powerpc/ofw/ (props changed) stable/9/sys/cddl/contrib/opensolaris/ (props changed) stable/9/sys/conf/ (props changed) stable/9/sys/contrib/dev/acpica/ (props changed) stable/9/sys/contrib/octeon-sdk/ (props changed) stable/9/sys/contrib/pf/ (props changed) stable/9/sys/contrib/x86emu/ (props changed) Modified: stable/9/sys/dev/ed/if_ed.c ============================================================================== --- stable/9/sys/dev/ed/if_ed.c Thu Jan 5 20:41:40 2012 (r229647) +++ stable/9/sys/dev/ed/if_ed.c Thu Jan 5 20:49:48 2012 (r229648) @@ -1709,12 +1709,19 @@ ed_shmem_write_mbufs(struct ed_softc *sc break; } } - for (len = 0; m != 0; m = m->m_next) { - if (sc->isa16bit) - bus_space_write_region_2(sc->mem_bst, - sc->mem_bsh, dst, - mtod(m, uint16_t *), (m->m_len + 1)/ 2); - else + for (len = 0; m != NULL; m = m->m_next) { + if (m->m_len == 0) + continue; + if (sc->isa16bit) { + if (m->m_len > 1) + bus_space_write_region_2(sc->mem_bst, + sc->mem_bsh, dst, + mtod(m, uint16_t *), m->m_len / 2); + if ((m->m_len & 1) != 0) + bus_space_write_1(sc->mem_bst, sc->mem_bsh, + dst + m->m_len - 1, + *(mtod(m, uint8_t *) + m->m_len - 1)); + } else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/162932: commit references a PR Date: Thu, 5 Jan 2012 21:17:25 +0000 (UTC) Author: yongari Date: Thu Jan 5 21:17:06 2012 New Revision: 229649 URL: http://svn.freebsd.org/changeset/base/229649 Log: MFC r228286: Fix off by one error in mbuf access. Previously it caused panic. While I'm here use NULL to compare mbuf pointer and add additional check for zero length mbuf before accessing the mbuf. PR: kern/162932 Modified: stable/8/sys/dev/ed/if_ed.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/dev/ed/if_ed.c ============================================================================== --- stable/8/sys/dev/ed/if_ed.c Thu Jan 5 20:49:48 2012 (r229648) +++ stable/8/sys/dev/ed/if_ed.c Thu Jan 5 21:17:06 2012 (r229649) @@ -1695,12 +1695,19 @@ ed_shmem_write_mbufs(struct ed_softc *sc break; } } - for (len = 0; m != 0; m = m->m_next) { - if (sc->isa16bit) - bus_space_write_region_2(sc->mem_bst, - sc->mem_bsh, dst, - mtod(m, uint16_t *), (m->m_len + 1)/ 2); - else + for (len = 0; m != NULL; m = m->m_next) { + if (m->m_len == 0) + continue; + if (sc->isa16bit) { + if (m->m_len > 1) + bus_space_write_region_2(sc->mem_bst, + sc->mem_bsh, dst, + mtod(m, uint16_t *), m->m_len / 2); + if ((m->m_len & 1) != 0) + bus_space_write_1(sc->mem_bst, sc->mem_bsh, + dst + m->m_len - 1, + *(mtod(m, uint8_t *) + m->m_len - 1)); + } else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/162932: commit references a PR Date: Thu, 5 Jan 2012 21:19:05 +0000 (UTC) Author: yongari Date: Thu Jan 5 21:18:34 2012 New Revision: 229650 URL: http://svn.freebsd.org/changeset/base/229650 Log: MFC r228286: Fix off by one error in mbuf access. Previously it caused panic. While I'm here use NULL to compare mbuf pointer and add additional check for zero length mbuf before accessing the mbuf. PR: kern/162932 Modified: stable/7/sys/dev/ed/if_ed.c Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/dev/ed/if_ed.c ============================================================================== --- stable/7/sys/dev/ed/if_ed.c Thu Jan 5 21:17:06 2012 (r229649) +++ stable/7/sys/dev/ed/if_ed.c Thu Jan 5 21:18:34 2012 (r229650) @@ -1689,12 +1689,19 @@ ed_shmem_write_mbufs(struct ed_softc *sc break; } } - for (len = 0; m != 0; m = m->m_next) { - if (sc->isa16bit) - bus_space_write_region_2(sc->mem_bst, - sc->mem_bsh, dst, - mtod(m, uint16_t *), (m->m_len + 1)/ 2); - else + for (len = 0; m != NULL; m = m->m_next) { + if (m->m_len == 0) + continue; + if (sc->isa16bit) { + if (m->m_len > 1) + bus_space_write_region_2(sc->mem_bst, + sc->mem_bsh, dst, + mtod(m, uint16_t *), m->m_len / 2); + if ((m->m_len & 1) != 0) + bus_space_write_1(sc->mem_bst, sc->mem_bsh, + dst + m->m_len - 1, + *(mtod(m, uint8_t *) + m->m_len - 1)); + } else bus_space_write_region_1(sc->mem_bst, sc->mem_bsh, dst, mtod(m, uint8_t *), m->m_len); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: patched->closed State-Changed-By: yongari State-Changed-When: Fri Jan 6 00:52:50 UTC 2012 State-Changed-Why: MFC to stable/[7-9] complete. Thanks a lot! http://www.freebsd.org/cgi/query-pr.cgi?pr=162932 >Unformatted: