From patchwork Mon Nov 16 12:36:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Timo Teras X-Patchwork-Id: 544990 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 98642141467 for ; Mon, 16 Nov 2015 23:36:58 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=kiX72Vqg; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752332AbbKPMgz (ORCPT ); Mon, 16 Nov 2015 07:36:55 -0500 Received: from mail-lf0-f45.google.com ([209.85.215.45]:34296 "EHLO mail-lf0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752305AbbKPMgv (ORCPT ); Mon, 16 Nov 2015 07:36:51 -0500 Received: by lffu14 with SMTP id u14so85766876lff.1 for ; Mon, 16 Nov 2015 04:36:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=hrfp/h3qsAuvUbRwREax0lF6X3bYqkSX+XLNUEZaEdo=; b=kiX72VqgWZl8CcLn/EOf6/v9cwyE90rXHjuHT8zRgRN2Nly29+CKJrCLGTD0sL8/LR eAxjNTIYhR1ai7H13ibGIOPtcTPZ9wkde6B7lo1YiFQm8Ze+aSXM5JL8Bp+oSgXstHFw m/92H3gg+68JJHMyp/LE+T7s+QExncnwMKfH6taXyCQzRH4EY+j9QNbNBrP3qgNBo/h2 u1ev/cZcu6dJqfJNUp0juQnXYcpGbE+2fmpsz0hChAGsUP2nO4nHNgPE5kstg+NSjFX+ BJekiQBeR87H7d5Vmu7gdEbphg6i39WKXaVydfaf5uzM8ZUs86q/8rrjNmqBRwUax8hI 9YfA== X-Received: by 10.25.35.194 with SMTP id j185mr16334459lfj.62.1447677409745; Mon, 16 Nov 2015 04:36:49 -0800 (PST) Received: from vostro.util.wtbts.net ([2001:1bc8:101:f402:21a:9fff:fe0c:4022]) by smtp.gmail.com with ESMTPSA id y79sm5539546lfd.45.2015.11.16.04.36.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 16 Nov 2015 04:36:48 -0800 (PST) From: =?UTF-8?q?Timo=20Ter=C3=A4s?= To: Francois Romieu , netdev@vger.kernel.org Cc: =?UTF-8?q?Timo=20Ter=C3=A4s?= Subject: [PATCH] via-velocity: unconditionally drop frames with bad l2 length Date: Mon, 16 Nov 2015 14:36:32 +0200 Message-Id: <1447677392-17400-1-git-send-email-timo.teras@iki.fi> X-Mailer: git-send-email 2.6.3 In-Reply-To: <20151113232133.GA21633@electric-eye.fr.zoreil.com> References: <20151113232133.GA21633@electric-eye.fr.zoreil.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org By default the driver allowed incorrect frames to be received. What is worse the code does not handle very short frames correctly. The FCS length is unconditionally subtracted, and the underflow can cause skb_put to be called with large number after implicit cast to unsigned. And indeed, an skb_over_panic() was observed with via-velocity. This removes the module parameter as it does not work in it's current state, and should be implemented via NETIF_F_RXALL if needed. Suggested-by: Francois Romieu Signed-off-by: Timo Teräs --- Francois, is this something like you had in mind? I can try give this a test spin in the known bad location, if this looks otherwise ok. drivers/net/ethernet/via/via-velocity.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/via/via-velocity.c b/drivers/net/ethernet/via/via-velocity.c index a43e849..03ce386 100644 --- a/drivers/net/ethernet/via/via-velocity.c +++ b/drivers/net/ethernet/via/via-velocity.c @@ -345,13 +345,6 @@ VELOCITY_PARAM(flow_control, "Enable flow control ability"); */ VELOCITY_PARAM(speed_duplex, "Setting the speed and duplex mode"); -#define VAL_PKT_LEN_DEF 0 -/* ValPktLen[] is used for setting the checksum offload ability of NIC. - 0: Receive frame with invalid layer 2 length (Default) - 1: Drop frame with invalid layer 2 length -*/ -VELOCITY_PARAM(ValPktLen, "Receiving or Drop invalid 802.3 frame"); - #define WOL_OPT_DEF 0 #define WOL_OPT_MIN 0 #define WOL_OPT_MAX 7 @@ -494,7 +487,6 @@ static void velocity_get_options(struct velocity_opt *opts, int index, velocity_set_int_opt(&opts->flow_cntl, flow_control[index], FLOW_CNTL_MIN, FLOW_CNTL_MAX, FLOW_CNTL_DEF, "flow_control", devname); velocity_set_bool_opt(&opts->flags, IP_byte_align[index], IP_ALIG_DEF, VELOCITY_FLAGS_IP_ALIGN, "IP_byte_align", devname); - velocity_set_bool_opt(&opts->flags, ValPktLen[index], VAL_PKT_LEN_DEF, VELOCITY_FLAGS_VAL_PKT_LEN, "ValPktLen", devname); velocity_set_int_opt((int *) &opts->spd_dpx, speed_duplex[index], MED_LNK_MIN, MED_LNK_MAX, MED_LNK_DEF, "Media link mode", devname); velocity_set_int_opt(&opts->wol_opts, wol_opts[index], WOL_OPT_MIN, WOL_OPT_MAX, WOL_OPT_DEF, "Wake On Lan options", devname); opts->numrx = (opts->numrx & ~3); @@ -2055,8 +2047,9 @@ static int velocity_receive_frame(struct velocity_info *vptr, int idx) int pkt_len = le16_to_cpu(rd->rdesc0.len) & 0x3fff; struct sk_buff *skb; - if (rd->rdesc0.RSR & (RSR_STP | RSR_EDP)) { - VELOCITY_PRT(MSG_LEVEL_VERBOSE, KERN_ERR " %s : the received frame spans multiple RDs.\n", vptr->netdev->name); + if (unlikely(rd->rdesc0.RSR & (RSR_STP | RSR_EDP | RSR_RL))) { + if (rd->rdesc0.RSR & (RSR_STP | RSR_EDP)) + VELOCITY_PRT(MSG_LEVEL_VERBOSE, KERN_ERR " %s : the received frame spans multiple RDs.\n", vptr->netdev->name); stats->rx_length_errors++; return -EINVAL; } @@ -2069,17 +2062,6 @@ static int velocity_receive_frame(struct velocity_info *vptr, int idx) dma_sync_single_for_cpu(vptr->dev, rd_info->skb_dma, vptr->rx.buf_sz, DMA_FROM_DEVICE); - /* - * Drop frame not meeting IEEE 802.3 - */ - - if (vptr->flags & VELOCITY_FLAGS_VAL_PKT_LEN) { - if (rd->rdesc0.RSR & RSR_RL) { - stats->rx_length_errors++; - return -EINVAL; - } - } - velocity_rx_csum(rd, skb); if (velocity_rx_copy(&skb, pkt_len, vptr) < 0) {