kern/176369: [PATCH] mxge: Correct some problem and do some cleanups
Andrew Gallatin
gallatin at cs.duke.edu
Sat Feb 23 12:30:02 UTC 2013
The following reply was made to PR kern/176369; it has been noted by GNATS.
From: Andrew Gallatin <gallatin at cs.duke.edu>
To: Christoph Mallon <christoph.mallon at gmx.de>
Cc: FreeBSD-gnats-submit at 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 at 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 */
>
More information about the freebsd-bugs
mailing list