From owner-freebsd-bugs@FreeBSD.ORG Sat Feb 23 12:30:02 2013 Return-Path: Delivered-To: freebsd-bugs@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 280CBBAA for ; Sat, 23 Feb 2013 12:30:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id 19306B5C for ; Sat, 23 Feb 2013 12:30:02 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.6/8.14.6) with ESMTP id r1NCU09Z093744 for ; Sat, 23 Feb 2013 12:30:00 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.6/8.14.6/Submit) id r1NCU0BP093739; Sat, 23 Feb 2013 12:30:00 GMT (envelope-from gnats) Date: Sat, 23 Feb 2013 12:30:00 GMT Message-Id: <201302231230.r1NCU0BP093739@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Andrew Gallatin Subject: Re: kern/176369: [PATCH] mxge: Correct some problem and do some cleanups X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Andrew Gallatin List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Feb 2013 12:30:02 -0000 The following reply was made to PR kern/176369; it has been noted by GNATS. From: Andrew Gallatin To: Christoph Mallon Cc: FreeBSD-gnats-submit@freebsd.org Subject: Re: kern/176369: [PATCH] mxge: Correct some problem and do some cleanups Date: Sat, 23 Feb 2013 07:24:20 -0500 Thanks. These mostly look good. I will test & apply them when I get to the office on Monday. Drew On 02/23/13 06:35, Christoph Mallon wrote: >> Number: 176369 >> Category: kern >> Synopsis: [PATCH] mxge: Correct some problem and do some cleanups >> Confidential: no >> Severity: non-critical >> Priority: low >> Responsible: freebsd-bugs >> State: open >> Quarter: >> Keywords: >> Date-Required: >> Class: update >> Submitter-Id: current-users >> Arrival-Date: Sat Feb 23 11:40:00 UTC 2013 >> Closed-Date: >> Last-Modified: >> Originator: Christoph Mallon >> Release: >> Organization: >> Environment: > > > >> Description: > This patch series corrects some problems (potential buffer overruns) and performs some cleanups in the mxge driver. > - Remove pointless null pointer tests after malloc(..., M_WAITOK). > - Use strncmp() instead of memcmp(). > - Use strlcpy() instead of the error-prone strncpy(). > - Remove the unused unused union qualhack. > - Check the MAC address in the EEPROM string more strictly. > - Expand the macro MXGE_NEXT_STRING() at its only user. > - Remove unnecessary buffer limit check. >> How-To-Repeat: > >> Fix: > Please apply these patches. > > --- 0001-mxge-Remove-pointless-null-pointer-tests-after-mallo.patch begins here --- >>From e65e89fece12ea9ac5b353875e366a103f71e479 Mon Sep 17 00:00:00 2001 > From: Christoph Mallon > Date: Sat, 23 Feb 2013 11:06:56 +0100 > Subject: [PATCH 1/7] mxge: Remove pointless null pointer tests after > malloc(..., M_WAITOK). > > Some cases would have bogusly returned 0 as error code anyway. > --- > sys/dev/mxge/if_mxge.c | 16 ---------------- > 1 file changed, 16 deletions(-) > > diff --git a/sys/dev/mxge/if_mxge.c b/sys/dev/mxge/if_mxge.c > index 245c139..a8c8aa4 100644 > --- a/sys/dev/mxge/if_mxge.c > +++ b/sys/dev/mxge/if_mxge.c > @@ -3325,8 +3325,6 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries, > size_t bytes; > int err, i; > > - err = ENOMEM; > - > /* allocate per-slice receive resources */ > > ss->rx_small.mask = ss->rx_big.mask = rx_ring_entries - 1; > @@ -3335,24 +3333,16 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries, > /* allocate the rx shadow rings */ > bytes = rx_ring_entries * sizeof (*ss->rx_small.shadow); > ss->rx_small.shadow = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK); > - if (ss->rx_small.shadow == NULL) > - return err; > > bytes = rx_ring_entries * sizeof (*ss->rx_big.shadow); > ss->rx_big.shadow = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK); > - if (ss->rx_big.shadow == NULL) > - return err; > > /* allocate the rx host info rings */ > bytes = rx_ring_entries * sizeof (*ss->rx_small.info); > ss->rx_small.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK); > - if (ss->rx_small.info == NULL) > - return err; > > bytes = rx_ring_entries * sizeof (*ss->rx_big.info); > ss->rx_big.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK); > - if (ss->rx_big.info == NULL) > - return err; > > /* allocate the rx busdma resources */ > err = bus_dma_tag_create(sc->parent_dmat, /* parent */ > @@ -3449,8 +3439,6 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries, > bytes = 8 + > sizeof (*ss->tx.req_list) * (ss->tx.max_desc + 4); > ss->tx.req_bytes = malloc(bytes, M_DEVBUF, M_WAITOK); > - if (ss->tx.req_bytes == NULL) > - return err; > /* ensure req_list entries are aligned to 8 bytes */ > ss->tx.req_list = (mcp_kreq_ether_send_t *) > ((unsigned long)(ss->tx.req_bytes + 7) & ~7UL); > @@ -3459,14 +3447,10 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries, > bytes = sizeof (*ss->tx.seg_list) * ss->tx.max_desc; > ss->tx.seg_list = (bus_dma_segment_t *) > malloc(bytes, M_DEVBUF, M_WAITOK); > - if (ss->tx.seg_list == NULL) > - return err; > > /* allocate the tx host info ring */ > bytes = tx_ring_entries * sizeof (*ss->tx.info); > ss->tx.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK); > - if (ss->tx.info == NULL) > - return err; > > /* allocate the tx busdma resources */ > err = bus_dma_tag_create(sc->parent_dmat, /* parent */ >