From patchwork Tue Jan 19 13:58:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xie He X-Patchwork-Id: 12031103 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E543AC433E0 for ; Tue, 19 Jan 2021 23:52:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9327522E01 for ; Tue, 19 Jan 2021 23:52:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729095AbhASXvy (ORCPT ); Tue, 19 Jan 2021 18:51:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46134 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406447AbhASN7h (ORCPT ); Tue, 19 Jan 2021 08:59:37 -0500 Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74049C061757; Tue, 19 Jan 2021 05:58:56 -0800 (PST) Received: by mail-pg1-x531.google.com with SMTP id z21so13014804pgj.4; Tue, 19 Jan 2021 05:58:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=imr5aG4KBQlZa6yz2Ssig4aeDe7tfo+aHQptqf2OYho=; b=GXQHG2Bu15eLLgG4HTjYnF/n7O4g6ZDSTIh0oeNKaaYZfxLPnqhP//g8zkBDeCIuj8 TOzpnq620vhYbHbq6PLb5yiF7dqomCcVxWXNsvjppSlvBwKN5tGaef1gNprE68oxT+QU pmpdhoy2edmTTAEb3tC8buqGuFP2h573Q/zWGtBReacFnEoyuoVNNsHe8gLRG97gGN4R Gh5T3IlozDR5Km6/WPOvNcn5prcD6ogIZoL7qUEO545G3l4HM3FLL3KKsJQXcnxA0787 85BesBUoYSVo8vVSj/20tYVpMhWK3l5tZQV7c3C3utdfocMT+X8ga2Eh5TqaY321FbjI Iwvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=imr5aG4KBQlZa6yz2Ssig4aeDe7tfo+aHQptqf2OYho=; b=ZJdBkcuTHxwXkEnRdYy5jfbyJErQxr7GT7kuMjF6m+4x6YmJOYYrWb7mT0xFq0f6cu 5x8JZZ8Ydkt1vq2iq2Xe77JcQp75POD5OQYbQGqAGr5S2Gke98Bu6EJQtVFRRWoJfvbR 5PX3ftWd6uS5x7MM1QGSywOOK0gZp/iDfC7441O5otSR6leGGOmsf07KXjxiWM9GULaS DOLuFsH9JppkBYFzri5+dg5UZ5riaXoTHxZR0FvUVWF4DlWLy2Q4CORf3HTvg43IgCO0 7Q+yhJQUgOStegEVGNTZyl6GyEW0LEzqZ6si3kQq4SeWb0tyr7loQGuzD5f89gZxjh5i 5dFg== X-Gm-Message-State: AOAM5301W13YA0Ocx5u6VrFVnKF7gQthzutiDHR49LZtuiaGS7lxGe0Q bn6LDmqsk6+7K8TbziAT4Yo= X-Google-Smtp-Source: ABdhPJzTy44RdMEeR0nuYkgoc+KLeotB4MquBnNKHCTzz1FHoe8q1SZx/iM83r5RonejFHFSuHyD7Q== X-Received: by 2002:a62:e30e:0:b029:1b9:3823:4b3a with SMTP id g14-20020a62e30e0000b02901b938234b3amr4177447pfh.15.1611064735972; Tue, 19 Jan 2021 05:58:55 -0800 (PST) Received: from shane-XPS-13-9380.hsd1.ca.comcast.net ([2601:646:8800:1c00:4d24:98ed:b117:fdd6]) by smtp.gmail.com with ESMTPSA id j17sm18577470pfh.183.2021.01.19.05.58.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Jan 2021 05:58:55 -0800 (PST) From: Xie He To: "David S. Miller" , Jakub Kicinski , linux-x25@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Martin Schiller Cc: Xie He Subject: [PATCH net v3] net: lapb: Add locking to the lapb module Date: Tue, 19 Jan 2021 05:58:19 -0800 Message-Id: <20210119135819.98140-1-xie.he.0141@gmail.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org In the lapb module, the timers may run concurrently with other code in this module, and there is currently no locking to prevent the code from racing on "struct lapb_cb". This patch adds locking to prevent racing. 1. Add "spinlock_t lock" to "struct lapb_cb"; Add "spin_lock_bh" and "spin_unlock_bh" to APIs, timer functions and notifier functions. 2. Add "bool t1timer_stop, t2timer_stop" to "struct lapb_cb" to make us able to ask running timers to abort; Modify "lapb_stop_t1timer" and "lapb_stop_t2timer" to make them able to abort running timers; Modify "lapb_t2timer_expiry" and "lapb_t1timer_expiry" to make them abort after they are stopped by "lapb_stop_t1timer", "lapb_stop_t2timer", and "lapb_start_t1timer", "lapb_start_t2timer". 3. In lapb_unregister, change "lapb_stop_t1timer" and "lapb_stop_t2timer" to "del_timer_sync" to make sure all running timers have exited. 4. The lapb_device_event function calls lapb_disconnect_request. In order to avoid trying to hold the lock twice, add a new function named "__lapb_disconnect_request" which assumes the lock is held, and make it called by lapb_disconnect_request and lapb_device_event. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie He --- include/net/lapb.h | 2 ++ net/lapb/lapb_iface.c | 62 ++++++++++++++++++++++++++++++------------- net/lapb/lapb_timer.c | 30 ++++++++++++++++++--- 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/include/net/lapb.h b/include/net/lapb.h index ccc3d1f020b0..eee73442a1ba 100644 --- a/include/net/lapb.h +++ b/include/net/lapb.h @@ -92,6 +92,7 @@ struct lapb_cb { unsigned short n2, n2count; unsigned short t1, t2; struct timer_list t1timer, t2timer; + bool t1timer_stop, t2timer_stop; /* Internal control information */ struct sk_buff_head write_queue; @@ -103,6 +104,7 @@ struct lapb_cb { struct lapb_frame frmr_data; unsigned char frmr_type; + spinlock_t lock; refcount_t refcnt; }; diff --git a/net/lapb/lapb_iface.c b/net/lapb/lapb_iface.c index 40961889e9c0..4de2edbe833a 100644 --- a/net/lapb/lapb_iface.c +++ b/net/lapb/lapb_iface.c @@ -122,6 +122,8 @@ static struct lapb_cb *lapb_create_cb(void) timer_setup(&lapb->t1timer, NULL, 0); timer_setup(&lapb->t2timer, NULL, 0); + lapb->t1timer_stop = true; + lapb->t2timer_stop = true; lapb->t1 = LAPB_DEFAULT_T1; lapb->t2 = LAPB_DEFAULT_T2; @@ -129,6 +131,8 @@ static struct lapb_cb *lapb_create_cb(void) lapb->mode = LAPB_DEFAULT_MODE; lapb->window = LAPB_DEFAULT_WINDOW; lapb->state = LAPB_STATE_0; + + spin_lock_init(&lapb->lock); refcount_set(&lapb->refcnt, 1); out: return lapb; @@ -178,8 +182,8 @@ int lapb_unregister(struct net_device *dev) goto out; lapb_put(lapb); - lapb_stop_t1timer(lapb); - lapb_stop_t2timer(lapb); + del_timer_sync(&lapb->t1timer); + del_timer_sync(&lapb->t2timer); lapb_clear_queues(lapb); @@ -201,6 +205,8 @@ int lapb_getparms(struct net_device *dev, struct lapb_parms_struct *parms) if (!lapb) goto out; + spin_lock_bh(&lapb->lock); + parms->t1 = lapb->t1 / HZ; parms->t2 = lapb->t2 / HZ; parms->n2 = lapb->n2; @@ -219,6 +225,7 @@ int lapb_getparms(struct net_device *dev, struct lapb_parms_struct *parms) else parms->t2timer = (lapb->t2timer.expires - jiffies) / HZ; + spin_unlock_bh(&lapb->lock); lapb_put(lapb); rc = LAPB_OK; out: @@ -234,6 +241,8 @@ int lapb_setparms(struct net_device *dev, struct lapb_parms_struct *parms) if (!lapb) goto out; + spin_lock_bh(&lapb->lock); + rc = LAPB_INVALUE; if (parms->t1 < 1 || parms->t2 < 1 || parms->n2 < 1) goto out_put; @@ -256,6 +265,7 @@ int lapb_setparms(struct net_device *dev, struct lapb_parms_struct *parms) rc = LAPB_OK; out_put: + spin_unlock_bh(&lapb->lock); lapb_put(lapb); out: return rc; @@ -270,6 +280,8 @@ int lapb_connect_request(struct net_device *dev) if (!lapb) goto out; + spin_lock_bh(&lapb->lock); + rc = LAPB_OK; if (lapb->state == LAPB_STATE_1) goto out_put; @@ -285,24 +297,18 @@ int lapb_connect_request(struct net_device *dev) rc = LAPB_OK; out_put: + spin_unlock_bh(&lapb->lock); lapb_put(lapb); out: return rc; } EXPORT_SYMBOL(lapb_connect_request); -int lapb_disconnect_request(struct net_device *dev) +static int __lapb_disconnect_request(struct lapb_cb *lapb) { - struct lapb_cb *lapb = lapb_devtostruct(dev); - int rc = LAPB_BADTOKEN; - - if (!lapb) - goto out; - switch (lapb->state) { case LAPB_STATE_0: - rc = LAPB_NOTCONNECTED; - goto out_put; + return LAPB_NOTCONNECTED; case LAPB_STATE_1: lapb_dbg(1, "(%p) S1 TX DISC(1)\n", lapb->dev); @@ -310,12 +316,10 @@ int lapb_disconnect_request(struct net_device *dev) lapb_send_control(lapb, LAPB_DISC, LAPB_POLLON, LAPB_COMMAND); lapb->state = LAPB_STATE_0; lapb_start_t1timer(lapb); - rc = LAPB_NOTCONNECTED; - goto out_put; + return LAPB_NOTCONNECTED; case LAPB_STATE_2: - rc = LAPB_OK; - goto out_put; + return LAPB_OK; } lapb_clear_queues(lapb); @@ -328,8 +332,22 @@ int lapb_disconnect_request(struct net_device *dev) lapb_dbg(1, "(%p) S3 DISC(1)\n", lapb->dev); lapb_dbg(0, "(%p) S3 -> S2\n", lapb->dev); - rc = LAPB_OK; -out_put: + return LAPB_OK; +} + +int lapb_disconnect_request(struct net_device *dev) +{ + struct lapb_cb *lapb = lapb_devtostruct(dev); + int rc = LAPB_BADTOKEN; + + if (!lapb) + goto out; + + spin_lock_bh(&lapb->lock); + + rc = __lapb_disconnect_request(lapb); + + spin_unlock_bh(&lapb->lock); lapb_put(lapb); out: return rc; @@ -344,6 +362,8 @@ int lapb_data_request(struct net_device *dev, struct sk_buff *skb) if (!lapb) goto out; + spin_lock_bh(&lapb->lock); + rc = LAPB_NOTCONNECTED; if (lapb->state != LAPB_STATE_3 && lapb->state != LAPB_STATE_4) goto out_put; @@ -352,6 +372,7 @@ int lapb_data_request(struct net_device *dev, struct sk_buff *skb) lapb_kick(lapb); rc = LAPB_OK; out_put: + spin_unlock_bh(&lapb->lock); lapb_put(lapb); out: return rc; @@ -364,7 +385,9 @@ int lapb_data_received(struct net_device *dev, struct sk_buff *skb) int rc = LAPB_BADTOKEN; if (lapb) { + spin_lock_bh(&lapb->lock); lapb_data_input(lapb, skb); + spin_unlock_bh(&lapb->lock); lapb_put(lapb); rc = LAPB_OK; } @@ -435,6 +458,8 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event, if (!lapb) return NOTIFY_DONE; + spin_lock_bh(&lapb->lock); + switch (event) { case NETDEV_UP: lapb_dbg(0, "(%p) Interface up: %s\n", dev, dev->name); @@ -454,7 +479,7 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event, break; case NETDEV_GOING_DOWN: if (netif_carrier_ok(dev)) - lapb_disconnect_request(dev); + __lapb_disconnect_request(lapb); break; case NETDEV_DOWN: lapb_dbg(0, "(%p) Interface down: %s\n", dev, dev->name); @@ -489,6 +514,7 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event, break; } + spin_unlock_bh(&lapb->lock); lapb_put(lapb); return NOTIFY_DONE; } diff --git a/net/lapb/lapb_timer.c b/net/lapb/lapb_timer.c index baa247fe4ed0..0230b272b7d1 100644 --- a/net/lapb/lapb_timer.c +++ b/net/lapb/lapb_timer.c @@ -40,6 +40,7 @@ void lapb_start_t1timer(struct lapb_cb *lapb) lapb->t1timer.function = lapb_t1timer_expiry; lapb->t1timer.expires = jiffies + lapb->t1; + lapb->t1timer_stop = false; add_timer(&lapb->t1timer); } @@ -50,16 +51,19 @@ void lapb_start_t2timer(struct lapb_cb *lapb) lapb->t2timer.function = lapb_t2timer_expiry; lapb->t2timer.expires = jiffies + lapb->t2; + lapb->t2timer_stop = false; add_timer(&lapb->t2timer); } void lapb_stop_t1timer(struct lapb_cb *lapb) { + lapb->t1timer_stop = true; del_timer(&lapb->t1timer); } void lapb_stop_t2timer(struct lapb_cb *lapb) { + lapb->t2timer_stop = true; del_timer(&lapb->t2timer); } @@ -72,16 +76,31 @@ static void lapb_t2timer_expiry(struct timer_list *t) { struct lapb_cb *lapb = from_timer(lapb, t, t2timer); + spin_lock_bh(&lapb->lock); + if (timer_pending(&lapb->t2timer)) /* A new timer has been set up */ + goto out; + if (lapb->t2timer_stop) /* The timer has been stopped */ + goto out; + if (lapb->condition & LAPB_ACK_PENDING_CONDITION) { lapb->condition &= ~LAPB_ACK_PENDING_CONDITION; lapb_timeout_response(lapb); } + +out: + spin_unlock_bh(&lapb->lock); } static void lapb_t1timer_expiry(struct timer_list *t) { struct lapb_cb *lapb = from_timer(lapb, t, t1timer); + spin_lock_bh(&lapb->lock); + if (timer_pending(&lapb->t1timer)) /* A new timer has been set up */ + goto out; + if (lapb->t1timer_stop) /* The timer has been stopped */ + goto out; + switch (lapb->state) { /* @@ -108,7 +127,7 @@ static void lapb_t1timer_expiry(struct timer_list *t) lapb->state = LAPB_STATE_0; lapb_disconnect_indication(lapb, LAPB_TIMEDOUT); lapb_dbg(0, "(%p) S1 -> S0\n", lapb->dev); - return; + goto out; } else { lapb->n2count++; if (lapb->mode & LAPB_EXTENDED) { @@ -132,7 +151,7 @@ static void lapb_t1timer_expiry(struct timer_list *t) lapb->state = LAPB_STATE_0; lapb_disconnect_confirmation(lapb, LAPB_TIMEDOUT); lapb_dbg(0, "(%p) S2 -> S0\n", lapb->dev); - return; + goto out; } else { lapb->n2count++; lapb_dbg(1, "(%p) S2 TX DISC(1)\n", lapb->dev); @@ -150,7 +169,7 @@ static void lapb_t1timer_expiry(struct timer_list *t) lapb_stop_t2timer(lapb); lapb_disconnect_indication(lapb, LAPB_TIMEDOUT); lapb_dbg(0, "(%p) S3 -> S0\n", lapb->dev); - return; + goto out; } else { lapb->n2count++; lapb_requeue_frames(lapb); @@ -167,7 +186,7 @@ static void lapb_t1timer_expiry(struct timer_list *t) lapb->state = LAPB_STATE_0; lapb_disconnect_indication(lapb, LAPB_TIMEDOUT); lapb_dbg(0, "(%p) S4 -> S0\n", lapb->dev); - return; + goto out; } else { lapb->n2count++; lapb_transmit_frmr(lapb); @@ -176,4 +195,7 @@ static void lapb_t1timer_expiry(struct timer_list *t) } lapb_start_t1timer(lapb); + +out: + spin_unlock_bh(&lapb->lock); }