Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751815AbdLEURX (ORCPT ); Tue, 5 Dec 2017 15:17:23 -0500 Received: from mx0a-00010702.pphosted.com ([148.163.156.75]:47082 "EHLO mx0b-00010702.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751394AbdLEURT (ORCPT ); Tue, 5 Dec 2017 15:17:19 -0500 Date: Tue, 5 Dec 2017 14:17:11 -0600 From: Julia Cartwright To: Julia Lawall CC: Rafal Ozieblo , Nicolas Ferre , , , Subject: [PATCH 1/2] net: macb: reduce scope of rx_fs_lock-protected regions Message-ID: <20171205201711.GA18445@jcartwri.amer.corp.natinst.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.1 (2017-09-22) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-05_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=inbound_policy_notspam policy=inbound_policy score=30 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=30 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1712050290 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3725 Lines: 113 Commit ae8223de3df5 ("net: macb: Added support for RX filtering") introduces a lock, rx_fs_lock which is intended to protect the list of rx_flow items and synchronize access to the hardware rx filtering registers. However, the region protected by this lock is overscoped, unnecessarily including things like slab allocation. Reduce this lock scope to only include operations which must be performed atomically: list traversal, addition, and removal, and hitting the macb filtering registers. This fixes the use of kmalloc w/ GFP_KERNEL in atomic context. Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering") Cc: Rafal Ozieblo Cc: Julia Lawall Signed-off-by: Julia Cartwright --- While Julia Lawall's cocci-generated patch fixes the problem, the right solution is to obviate the problem altogether. Thanks, The Other Julia drivers/net/ethernet/cadence/macb_main.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index c5fa87cdc6c4..e7ef104a077d 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2796,6 +2796,7 @@ static int gem_add_flow_filter(struct net_device *netdev, struct macb *bp = netdev_priv(netdev); struct ethtool_rx_flow_spec *fs = &cmd->fs; struct ethtool_rx_fs_item *item, *newfs; + unsigned long flags; int ret = -EINVAL; bool added = false; @@ -2811,6 +2812,8 @@ static int gem_add_flow_filter(struct net_device *netdev, htonl(fs->h_u.tcp_ip4_spec.ip4dst), htons(fs->h_u.tcp_ip4_spec.psrc), htons(fs->h_u.tcp_ip4_spec.pdst)); + spin_lock_irqsave(&bp->rx_fs_lock, flags); + /* find correct place to add in list */ if (list_empty(&bp->rx_fs_list.list)) list_add(&newfs->list, &bp->rx_fs_list.list); @@ -2837,9 +2840,11 @@ static int gem_add_flow_filter(struct net_device *netdev, if (netdev->features & NETIF_F_NTUPLE) gem_enable_flow_filters(bp, 1); + spin_unlock_irqrestore(&bp->rx_fs_lock, flags); return 0; err: + spin_unlock_irqrestore(&bp->rx_fs_lock, flags); kfree(newfs); return ret; } @@ -2850,9 +2855,14 @@ static int gem_del_flow_filter(struct net_device *netdev, struct macb *bp = netdev_priv(netdev); struct ethtool_rx_fs_item *item; struct ethtool_rx_flow_spec *fs; + unsigned long flags; - if (list_empty(&bp->rx_fs_list.list)) + spin_lock_irqsave(&bp->rx_fs_lock, flags); + + if (list_empty(&bp->rx_fs_list.list)) { + spin_unlock_irqrestore(&bp->rx_fs_lock, flags); return -EINVAL; + } list_for_each_entry(item, &bp->rx_fs_list.list, list) { if (item->fs.location == cmd->fs.location) { @@ -2869,12 +2879,14 @@ static int gem_del_flow_filter(struct net_device *netdev, gem_writel_n(bp, SCRT2, fs->location, 0); list_del(&item->list); - kfree(item); bp->rx_fs_list.count--; + spin_unlock_irqrestore(&bp->rx_fs_lock, flags); + kfree(item); return 0; } } + spin_unlock_irqrestore(&bp->rx_fs_lock, flags); return -EINVAL; } @@ -2943,11 +2955,8 @@ static int gem_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd, static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd) { struct macb *bp = netdev_priv(netdev); - unsigned long flags; int ret; - spin_lock_irqsave(&bp->rx_fs_lock, flags); - switch (cmd->cmd) { case ETHTOOL_SRXCLSRLINS: if ((cmd->fs.location >= bp->max_tuples) @@ -2966,7 +2975,6 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd) ret = -EOPNOTSUPP; } - spin_unlock_irqrestore(&bp->rx_fs_lock, flags); return ret; } -- 2.14.2