From owner-freebsd-bugs Tue Oct 1 07:40:06 1996 Return-Path: owner-bugs Received: (from root@localhost) by freefall.freebsd.org (8.7.5/8.7.3) id HAA20786 for bugs-outgoing; Tue, 1 Oct 1996 07:40:06 -0700 (PDT) Received: (from gnats@localhost) by freefall.freebsd.org (8.7.5/8.7.3) id HAA20756; Tue, 1 Oct 1996 07:40:03 -0700 (PDT) Resent-Date: Tue, 1 Oct 1996 07:40:03 -0700 (PDT) Resent-Message-Id: <199610011440.HAA20756@freefall.freebsd.org> Resent-From: gnats (GNATS Management) Resent-To: freebsd-bugs Resent-Reply-To: FreeBSD-gnats@freefall.FreeBSD.org, seki@sysrap.cs.fujitsu.co.jp Received: from fgwmail.fujitsu.co.jp (fgwmail.fujitsu.co.jp [164.71.1.133]) by freefall.freebsd.org (8.7.5/8.7.3) with ESMTP id HAA20365 for ; Tue, 1 Oct 1996 07:34:58 -0700 (PDT) Received: from fdmmail.fujitsu.co.jp by fgwmail.fujitsu.co.jp (8.7.6+2.6Wbeta7/3.3W5-MX960815-Fujitsu Mail Gateway) id XAA16923; Tue, 1 Oct 1996 23:34:55 +0900 (JST) Received: from axis.sysrap.cs.fujitsu.co.jp by fdmmail.fujitsu.co.jp (8.6.12+2.5Wb4/3.3W9-MX960926-Fujitsu Domain Mail Master) id XAA27897; Tue, 1 Oct 1996 23:34:22 +0900 Received: (from seki@localhost) by axis.sysrap.cs.fujitsu.co.jp (8.6.12/8.6.12.01) id XAA02892; Tue, 1 Oct 1996 23:30:37 +0900 Message-Id: <199610011430.XAA02892@axis.sysrap.cs.fujitsu.co.jp> Date: Tue, 1 Oct 1996 23:30:37 +0900 From: seki@sysrap.cs.fujitsu.co.jp Reply-To: seki@sysrap.cs.fujitsu.co.jp To: FreeBSD-gnats-submit@freebsd.org Cc: seki@sysrap.cs.fujitsu.co.jp X-Send-Pr-Version: 3.2 Subject: i386/1701: packet padding bug in if_fe driver Sender: owner-bugs@freebsd.org X-Loop: FreeBSD.org Precedence: bulk >Number: 1701 >Category: i386 >Synopsis: padding code of short packets in fe driver is broken >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Oct 1 07:40:01 PDT 1996 >Last-Modified: >Originator: Masahiro SEKIGUCHI >Organization: Fujitsu Limited >Release: FreeBSD 2.2-960801-SNAP i386 >Environment: FreeBSD 960801 SNAP or later Fe compatible Ethrenet cards (FMV-180, AT1700, or RE2000) >Description: Fe driver formerly had a code to pad short packets to Ethernet minimum length upon transmission. That part of the code is missing in the if_fe.c in the 960801 SNAP or later. In the worst case, the driver may send some garbage packets with random Ethernet destination address. >How-To-Repeat: I found the bug when reading the source program, and I don't know exact way how I can cause the above behaviour. >Fix: I'm sending two patches. One is for 960801 SNAP (if_fe.c 1.16) and another for -current (if_fe.c 1.17). The latter (for -current) also includes another fix for a trivial bug only in the version. ------ cut here ------ cut here ------ cut here ------ For if_fe.c 1.16 (in 960801 SNAP) --- if_fe.c.old Tue Oct 1 21:03:09 1996 +++ if_fe.c Tue Oct 1 21:33:33 1996 @@ -227,7 +227,6 @@ u_short txb_free; /* free bytes in TX buffer */ u_char txb_count; /* number of packets in TX buffer */ u_char txb_sched; /* number of scheduled packets */ - u_char txb_padding; /* number of delayed padding bytes */ /* Multicast address filter management. */ u_char filter_change; /* MARs must be changed ASAP. */ @@ -2617,14 +2616,6 @@ * Packets shorter than Ethernet minimum are legal, and we pad them * before sending out. An exception is "partial" packets which are * shorter than mandatory Ethernet header. - * - * I wrote a code for an experimental "delayed padding" technique. - * When employed, it postpones the padding process for short packets. - * If xmit() occurred at the moment, the padding process is omitted, and - * garbage is sent as pad data. If next packet is stored in the - * transmission buffer before xmit(), write_mbuf() pads the previous - * packet before transmitting new packet. This *may* gain the - * system performance (slightly). */ static void fe_write_mbufs ( struct fe_softc *sc, struct mbuf *m ) @@ -2636,6 +2627,8 @@ u_short savebyte; /* WARNING: Architecture dependent! */ #define NO_PENDING_BYTE 0xFFFF + static u_char padding [ ETHER_MIN_LEN - ETHER_HDR_SIZE ]; + #if FE_DEBUG >= 2 /* First, count up the total number of bytes to copy */ length = 0; @@ -2726,6 +2719,11 @@ /* Spit the last byte, if the length is odd. */ if ( savebyte != NO_PENDING_BYTE ) { outw( addr_bmpr8, savebyte ); + } + + /* Pad to the Ethernet minimum length, if the packet is too short. */ + if ( length < ETHER_MIN_LEN ) { + outsw( addr_bmpr8, padding, ( ETHER_MIN_LEN - length ) >> 1); } } ------ cut here ------ cut here ------ cut here ------ For if_fe.c 1.17 (in -current) --- if_fe.c.old Tue Aug 6 12:15:00 1996 +++ if_fe.c Tue Oct 1 22:00:32 1996 @@ -227,7 +227,6 @@ u_short txb_free; /* free bytes in TX buffer */ u_char txb_count; /* number of packets in TX buffer */ u_char txb_sched; /* number of scheduled packets */ - u_char txb_padding; /* number of delayed padding bytes */ /* Multicast address filter management. */ u_char filter_change; /* MARs must be changed ASAP. */ @@ -2609,14 +2608,6 @@ * Packets shorter than Ethernet minimum are legal, and we pad them * before sending out. An exception is "partial" packets which are * shorter than mandatory Ethernet header. - * - * I wrote a code for an experimental "delayed padding" technique. - * When employed, it postpones the padding process for short packets. - * If xmit() occurred at the moment, the padding process is omitted, and - * garbage is sent as pad data. If next packet is stored in the - * transmission buffer before xmit(), write_mbuf() pads the previous - * packet before transmitting new packet. This *may* gain the - * system performance (slightly). */ static void fe_write_mbufs ( struct fe_softc *sc, struct mbuf *m ) @@ -2628,6 +2619,8 @@ u_short savebyte; /* WARNING: Architecture dependent! */ #define NO_PENDING_BYTE 0xFFFF + static u_char padding [ ETHER_MIN_LEN - ETHER_CRC_LEN - ETHER_HDR_LEN ]; + #if FE_DEBUG >= 2 /* First, count up the total number of bytes to copy */ length = 0; @@ -2650,9 +2643,9 @@ * it should be a bug of upper layer. We just ignore it. * ... Partial (too short) packets, neither. */ - if ( ETHER_IS_VALID_LEN(length + ETHER_CRC_LEN)) { + if ( ! ETHER_IS_VALID_LEN(length + ETHER_CRC_LEN)) { log( LOG_ERR, - "fe%d: got a out-of-spes packet (%u bytes) to send\n", + "fe%d: got an out-of-spec packet (%u bytes) to send\n", sc->sc_unit, length ); sc->sc_if.if_oerrors++; return; @@ -2717,6 +2710,11 @@ /* Spit the last byte, if the length is odd. */ if ( savebyte != NO_PENDING_BYTE ) { outw( addr_bmpr8, savebyte ); + } + + /* Pad to the Ethernet minimum length, if the packet is too short. */ + if ( length < ETHER_MIN_LEN - ETHER_CRC_LEN ) { + outsw( addr_bmpr8, padding, ( ETHER_MIN_LEN - ETHER_CRC_LEN - length ) >> 1); } } ------ cut here ------ cut here ------ cut here ------ >Audit-Trail: >Unformatted: