From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932498AbdJ0ULQ (ORCPT ); Fri, 27 Oct 2017 16:11:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38030 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbdJ0ULM (ORCPT ); Fri, 27 Oct 2017 16:11:12 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D412085550 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=longman@redhat.com From: Waiman Long To: Alexander Viro , Jan Kara , Jeff Layton , "J. Bruce Fields" , Tejun Heo , Christoph Lameter Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Andi Kleen , Dave Chinner , Boqun Feng , Davidlohr Bueso , Waiman Long Subject: [PATCH v7 10/10] lib/dlock-list: Fix use-after-unlock problem in dlist_for_each_entry_safe() Date: Fri, 27 Oct 2017 16:10:53 -0400 Message-Id: <1509135053-19214-1-git-send-email-longman@redhat.com> In-Reply-To: <1507229008-20569-1-git-send-email-longman@redhat.com> References: <1507229008-20569-1-git-send-email-longman@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 27 Oct 2017 20:11:12 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The dlist_for_each_entry_safe() macro in include/linux/dlock-list has a use-after-unlock problem where racing condition can happen because of a lack of spinlock protection. Fortunately, this macro is not currently being used in the kernel. This patch changes the dlist_for_each_entry_safe() macro so that the call to __dlock_list_next_list() is deferred until the next entry is being used. That should eliminate the use-after-unlock problem. Reported-by: Boqun Feng Signed-off-by: Waiman Long --- include/linux/dlock-list.h | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/include/linux/dlock-list.h b/include/linux/dlock-list.h index 02c5f4d..f4b7657 100644 --- a/include/linux/dlock-list.h +++ b/include/linux/dlock-list.h @@ -191,17 +191,17 @@ extern void dlock_list_add(struct dlock_list_node *node, } /** - * dlock_list_first_entry - get the first element from a list + * dlock_list_next_list_entry - get first element from next list in iterator * @iter : The dlock list iterator. - * @type : The type of the struct this is embedded in. + * @pos : A variable of the struct that is embedded in. * @member: The name of the dlock_list_node within the struct. - * Return : Pointer to the next entry or NULL if all the entries are iterated. + * Return : Pointer to first entry or NULL if all the lists are iterated. */ -#define dlock_list_first_entry(iter, type, member) \ +#define dlock_list_next_list_entry(iter, pos, member) \ ({ \ struct dlock_list_node *_n; \ _n = __dlock_list_next_entry(NULL, iter); \ - _n ? list_entry(_n, type, member) : NULL; \ + _n ? list_entry(_n, typeof(*pos), member) : NULL; \ }) /** @@ -231,7 +231,7 @@ extern void dlock_list_add(struct dlock_list_node *node, * This iteration function is designed to be used in a while loop. */ #define dlist_for_each_entry(pos, iter, member) \ - for (pos = dlock_list_first_entry(iter, typeof(*(pos)), member);\ + for (pos = dlock_list_next_list_entry(iter, pos, member); \ pos != NULL; \ pos = dlock_list_next_entry(pos, iter, member)) @@ -245,14 +245,20 @@ extern void dlock_list_add(struct dlock_list_node *node, * This iteration macro is safe with respect to list entry removal. * However, it cannot correctly iterate newly added entries right after the * current one. + * + * The call to __dlock_list_next_list() is deferred until the next entry + * is being iterated to avoid use-after-unlock problem. */ #define dlist_for_each_entry_safe(pos, n, iter, member) \ - for (pos = dlock_list_first_entry(iter, typeof(*(pos)), member);\ + for (pos = NULL; \ ({ \ - bool _b = (pos != NULL); \ - if (_b) \ - n = dlock_list_next_entry(pos, iter, member); \ - _b; \ + if (!pos || \ + (&(pos)->member.list == &(iter)->entry->list)) \ + pos = dlock_list_next_list_entry(iter, pos, \ + member); \ + if (pos) \ + n = list_next_entry(pos, member.list); \ + pos; \ }); \ pos = n) -- 1.8.3.1