From patchwork Wed Apr 24 01:19:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Linus_L=C3=BCssing?= X-Patchwork-Id: 17890 X-Patchwork-Delegate: sw@simonwunderlich.de Return-Path: X-Original-To: patchwork@open-mesh.org Delivered-To: patchwork@open-mesh.org Received: from open-mesh.org (localhost [IPv6:::1]) by open-mesh.org (Postfix) with ESMTP id D2A0681B10; Wed, 24 Apr 2019 03:19:28 +0200 (CEST) Authentication-Results: open-mesh.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=c0d3.blue header.i=@c0d3.blue header.b="UZaq8JCM"; dkim-atps=neutral Received-SPF: None (mailfrom) identity=mailfrom; client-ip=2a01:4f8:171:314c::100:a1; helo=mail.aperture-lab.de; envelope-from=linus.luessing@c0d3.blue; receiver= Received: from mail.aperture-lab.de (mail.aperture-lab.de [IPv6:2a01:4f8:171:314c::100:a1]) by open-mesh.org (Postfix) with ESMTPS id 6A6708098C for ; Wed, 24 Apr 2019 03:19:26 +0200 (CEST) From: =?utf-8?q?Linus_L=C3=BCssing?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=c0d3.blue; s=2018; t=1556068766; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/Q4HreET4C2fwnIkJCVFsUGJ+orG1h3Rl4vSeOVO624=; b=UZaq8JCMNFCgg5smD/pLVZMRHqJl/HfGTWeCb+KpsF+r6p6rqGtNCOULQnFAGjUA26UhdK uXCNyYpQx/4dQFTSImdYjPGRBOQLq9x7dciTn1zXeMvcOcq48+4DzmHIjFNE1JHpUK1cS2 8Y0DiivwaSJaM1qCwP/ul/Y0014JsM5Qsu2AQzapUHT3xpaLLKQ/MT5afa+dwtrdftHNLF TLY6sF8FFVcRruN+a+kCUduqWUV5V1ZG7Mrs6IBwWU2hNv2Ixw66VgmrUCwInKZ7l1JyM6 J4ikNe1feiE5ZtTYZ0ci9ZN1DdX6BUt+OisrRquimAwDxHlIwwhDXonGBZ9gyg== To: b.a.t.m.a.n@lists.open-mesh.org Date: Wed, 24 Apr 2019 03:19:14 +0200 Message-Id: <20190424011919.9821-2-linus.luessing@c0d3.blue> In-Reply-To: <20190424011919.9821-1-linus.luessing@c0d3.blue> References: <20190424011919.9821-1-linus.luessing@c0d3.blue> MIME-Version: 1.0 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=c0d3.blue; s=2018; t=1556068766; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/Q4HreET4C2fwnIkJCVFsUGJ+orG1h3Rl4vSeOVO624=; b=i4L53xfAeJ9DGhMpxWSWvv1jUXkXJAv1zNdihwgYkqrb4e9svusYEeDYp2rsZqMfczeukh Go9Z5C31twbxUyo3J5S4mQPj94XHHrff5tNW2+OhOzycCJZzHMtfw5i1BFMVV36Df0rIr0 /sDzVqOwZ7rEed36nxNQVMrp5IFNsHpK35yaCpNQNB2V+frY/O1DInNwf3gIyTIfV2iUiV 0mZscfpsWUaMw9Q1KX6+cRj+ogAjS6koChxlHguknuy+ku8n77Ao589uSYeT0QpaeSN0se AiGXkKGE7P8pSdRPk2AI+Iv6aM9TszepXbf36IDwu3/ldEQzwbIYM5h4SFA22g== ARC-Seal: i=1; s=2018; d=c0d3.blue; t=1556068766; a=rsa-sha256; cv=none; b=sX9LlG0ZacECQwPOi+D/5RIAWRjYi2QTzrkyQniHROon3ncWAPk2F1FCbYCWf/L0fOSTKR AT0pfdlnO3SEB8ijaNB9TJfI7im2lZO25xOce/C8HioSd5f7U55q8u7Qz2Ik+LTR9paHFT wqHXQ2T4hvhbxopOs16NxJu9sjWN4DGlbjgxx4bc2wTIrcp3J7wZuCkbA9KftJXKG3oKsC ivHBFj3sDpCWEPNjDXLzIK9L4/kURHiocTMi5RG77RLosNsfre8UpGCT19orna/5DTdCLb xv7FXGbzhIhEij+XScCLwhYFtDSCgkOaFhqoPoYyvZ+K4Pjp2z6UdDLib6s08A== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=linus.luessing@c0d3.blue smtp.mailfrom=linus.luessing@c0d3.blue Authentication-Results: ORIGINATING; auth=pass smtp.auth=linus.luessing@c0d3.blue smtp.mailfrom=linus.luessing@c0d3.blue Subject: [B.A.T.M.A.N.] [PATCH maint 1/6] batman-adv: mcast: fix multicast tt/tvlv worker locking X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking Errors-To: b.a.t.m.a.n-bounces@lists.open-mesh.org Sender: "B.A.T.M.A.N" Syzbot has reported some issues with the locking assumptions made for the multicast tt/tvlv worker: It was able to trigger the WARN_ON() in batadv_mcast_mla_tt_retract() and batadv_mcast_mla_tt_add(). While hard/not reproduceable for us so far it seems that the delayed_work_pending() we use might not be quite safe from reordering. Therefore this patch adds an explicit, new spinlock to protect the update of the mla_list and flags in bat_priv and then removes the WARN_ON(delayed_work_pending()). Reported-by: syzbot+83f2d54ec6b7e417e13f@syzkaller.appspotmail.com Reported-by: syzbot+050927a651272b145a5d@syzkaller.appspotmail.com Fixes: 40b384052672 ("batman-adv: Use own timer for multicast TT and TVLV updates") Signed-off-by: Linus Lüssing --- https://www.open-mesh.org/issues/369 https://www.open-mesh.org/issues/370 Remark: In the past, before 40b384052672, there things were actually explicitly locked under the tt.commit_lock. --- net/batman-adv/main.c | 1 + net/batman-adv/multicast.c | 11 +++-------- net/batman-adv/types.h | 5 +++++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index dabcaff8..4a89177d 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -168,6 +168,7 @@ int batadv_mesh_init(struct net_device *soft_iface) spin_lock_init(&bat_priv->tt.commit_lock); spin_lock_init(&bat_priv->gw.list_lock); #ifdef CONFIG_BATMAN_ADV_MCAST + spin_lock_init(&bat_priv->mcast.mla_lock); spin_lock_init(&bat_priv->mcast.want_lists_lock); #endif spin_lock_init(&bat_priv->tvlv.container_list_lock); diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c index 3feb9435..ec54e236 100644 --- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c @@ -314,8 +314,6 @@ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list) * translation table except the ones listed in the given mcast_list. * * If mcast_list is NULL then all are retracted. - * - * Do not call outside of the mcast worker! (or cancel mcast worker first) */ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, struct hlist_head *mcast_list) @@ -323,8 +321,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, struct batadv_hw_addr *mcast_entry; struct hlist_node *tmp; - WARN_ON(delayed_work_pending(&bat_priv->mcast.work)); - hlist_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list, list) { if (mcast_list && @@ -348,8 +344,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, * * Adds multicast listener announcements from the given mcast_list to the * translation table if they have not been added yet. - * - * Do not call outside of the mcast worker! (or cancel mcast worker first) */ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, struct hlist_head *mcast_list) @@ -357,8 +351,6 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, struct batadv_hw_addr *mcast_entry; struct hlist_node *tmp; - WARN_ON(delayed_work_pending(&bat_priv->mcast.work)); - if (!mcast_list) return; @@ -647,7 +639,10 @@ static void batadv_mcast_mla_update(struct work_struct *work) priv_mcast = container_of(delayed_work, struct batadv_priv_mcast, work); bat_priv = container_of(priv_mcast, struct batadv_priv, mcast); + spin_lock(&bat_priv->mcast.mla_lock); __batadv_mcast_mla_update(bat_priv); + spin_unlock(&bat_priv->mcast.mla_lock); + batadv_mcast_start_timer(bat_priv); } diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 357ca119..74b64473 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1211,6 +1211,11 @@ struct batadv_priv_mcast { /** @bridged: whether the soft interface has a bridge on top */ unsigned char bridged:1; + /** + * @mla_lock: a lock protecting mla_list and mla_flags + */ + spinlock_t mla_lock; + /** * @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP * traffic