Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756811AbcCaN66 (ORCPT ); Thu, 31 Mar 2016 09:58:58 -0400 Received: from mail-pf0-f178.google.com ([209.85.192.178]:32829 "EHLO mail-pf0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750751AbcCaN64 (ORCPT ); Thu, 31 Mar 2016 09:58:56 -0400 Subject: Re: Possible ABA in use of llist.h llist_del_first() in tty_buffer and ib_rdma To: Mathieu Desnoyers References: <513177315.41302.1459417186529.JavaMail.zimbra@efficios.com> <1934237589.41303.1459417221186.JavaMail.zimbra@efficios.com> Cc: Huang Ying , Greg Kroah-Hartman , "Paul E. McKenney" , Andi Kleen , "David S. Miller" , lkml From: Peter Hurley Message-ID: <56FD2D1B.2010208@hurleysoftware.com> Date: Thu, 31 Mar 2016 06:58:51 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1934237589.41303.1459417221186.JavaMail.zimbra@efficios.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2201 Lines: 68 Hi Mathieu, On 03/31/2016 02:40 AM, Mathieu Desnoyers wrote: > CCing LKML. > > ----- On Mar 31, 2016, at 5:39 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > >> Hi, >> >> Code review (really: grepping the Linux kernel for >> llist_del_first) leads me to notice two possible ABA issues. >> The first one is in drivers/tty/tty_buffer.c, and is due to >> its use of llist_del_all and llist_del_first without locking >> since commit 809850b7a5 "tty: Use lockless flip buffer free list". >> >> Unfortunately, it appears to do so without respecting the >> locking requirements associated with llist_del_first. >> >> Quoting llist.h: >> >> " * If there are multiple producers and one consumer, llist_add can be >> * used in producers and llist_del_all or llist_del_first can be used >> * in the consumer. The use of llist_del_all in tty_buffer_free_all() is not concurrent with any other use of the free list; the comments for tty_buffer_free_all() even note the special condition. Only the llist_del_first() and llist_add() usage are concurrent, and fwiw, that usage is single-producer/single-consumer. Regards, Peter Hurley >> * This can be summarized as follow: >> * >> * | add | del_first | del_all >> * add | - | - | - >> * del_first | | L | L >> * del_all | | | - >> * >> * Where "-" stands for no lock is needed, while "L" stands for lock >> * is needed. >> " >> >> As soon as a llist_del_first() is used, then both llist_del_first() >> and llist_del_all() need to be protected by a lock, thus preventing >> ABA in llist_del_first(). >> >> An alternative to locking would be to only use llist_del_all() and >> never llist_del_first(). >> >> I'm also noticing a similar concurrent use of llist_del_first() and >> llist_del_all() in commit 1bc144b625 "net, rds, Replace xlist in net/rds/xlist.h >> with llist". >> The locking surrounding their use (especially in rds_ib_reuse_mr) >> don't appear clearly documented there. Perhaps there was a preexisting >> issue with the xlist.h use too ? >> >> Thanks, >> >> Mathieu >> >> -- >> Mathieu Desnoyers >> EfficiOS Inc. >> http://www.efficios.com >