Date: Sat, 23 Feb 2013 12:30:00 GMT From: Andrew Gallatin <gallatin@cs.duke.edu> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/176369: [PATCH] mxge: Correct some problem and do some cleanups Message-ID: <201302231230.r1NCU0BP093739@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/176369; it has been noted by GNATS. From: Andrew Gallatin <gallatin@cs.duke.edu> To: Christoph Mallon <christoph.mallon@gmx.de> 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 <christoph.mallon@gmx.de> > 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 */ >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201302231230.r1NCU0BP093739>