From patchwork Fri Nov 6 10:23:01 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 37834 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.176.167]) by ozlabs.org (Postfix) with ESMTP id CC951B7B65 for ; Fri, 6 Nov 2009 21:23:10 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755324AbZKFKXA (ORCPT ); Fri, 6 Nov 2009 05:23:00 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754951AbZKFKW7 (ORCPT ); Fri, 6 Nov 2009 05:22:59 -0500 Received: from gw1.cosmosbay.com ([212.99.114.194]:37647 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754654AbZKFKW7 (ORCPT ); Fri, 6 Nov 2009 05:22:59 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id nA6AN16Y009399; Fri, 6 Nov 2009 11:23:01 +0100 Message-ID: <4AF3F905.4030608@gmail.com> Date: Fri, 06 Nov 2009 11:23:01 +0100 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.23 (Windows/20090812) MIME-Version: 1.0 To: "David S. Miller" CC: Linux Netdev List Subject: [PATCH] can: should not use __dev_get_by_index() without locks X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Fri, 06 Nov 2009 11:23:01 +0100 (CET) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org David A more elegant patch will be possible for 2.6.33, but for 2.6.32, I think following patch is needed (Please note I did not test it) (More elegant : use RCU lookups ;) , I'll wait for net-next-2.6 upgrade as well) Thanks [PATCH] can: should not use __dev_get_by_index() without locks bcm_proc_getifname() is called with RTNL and dev_base_lock not held. It calls __dev_get_by_index() without locks, and this is illegal (might crash) Close the race by holding dev_base_lock and copying dev->name in the protected section. Signed-off-by: Eric Dumazet Signed-off-by: Oliver Hartkopp --- net/can/bcm.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/can/bcm.c b/net/can/bcm.c index 597da4f..e8d58f3 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -132,23 +132,27 @@ static inline struct bcm_sock *bcm_sk(const struct sock *sk) /* * procfs functions */ -static char *bcm_proc_getifname(int ifindex) +static char *bcm_proc_getifname(char *result, int ifindex) { struct net_device *dev; if (!ifindex) return "any"; - /* no usage counting */ + read_lock(&dev_base_lock); dev = __dev_get_by_index(&init_net, ifindex); if (dev) - return dev->name; + strcpy(result, dev->name); + else + strcpy(result, "???"); + read_unlock(&dev_base_lock); - return "???"; + return result; } static int bcm_proc_show(struct seq_file *m, void *v) { + char ifname[IFNAMSIZ]; struct sock *sk = (struct sock *)m->private; struct bcm_sock *bo = bcm_sk(sk); struct bcm_op *op; @@ -157,7 +161,7 @@ static int bcm_proc_show(struct seq_file *m, void *v) seq_printf(m, " / sk %p", sk); seq_printf(m, " / bo %p", bo); seq_printf(m, " / dropped %lu", bo->dropped_usr_msgs); - seq_printf(m, " / bound %s", bcm_proc_getifname(bo->ifindex)); + seq_printf(m, " / bound %s", bcm_proc_getifname(ifname, bo->ifindex)); seq_printf(m, " <<<\n"); list_for_each_entry(op, &bo->rx_ops, list) { @@ -169,7 +173,7 @@ static int bcm_proc_show(struct seq_file *m, void *v) continue; seq_printf(m, "rx_op: %03X %-5s ", - op->can_id, bcm_proc_getifname(op->ifindex)); + op->can_id, bcm_proc_getifname(ifname, op->ifindex)); seq_printf(m, "[%d]%c ", op->nframes, (op->flags & RX_CHECK_DLC)?'d':' '); if (op->kt_ival1.tv64) @@ -194,7 +198,8 @@ static int bcm_proc_show(struct seq_file *m, void *v) list_for_each_entry(op, &bo->tx_ops, list) { seq_printf(m, "tx_op: %03X %s [%d] ", - op->can_id, bcm_proc_getifname(op->ifindex), + op->can_id, + bcm_proc_getifname(ifname, op->ifindex), op->nframes); if (op->kt_ival1.tv64)