Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754212AbbDPDwV (ORCPT ); Wed, 15 Apr 2015 23:52:21 -0400 Received: from SpacedOut.fries.net ([67.64.210.234]:46649 "EHLO SpacedOut.fries.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753189AbbDPDwO (ORCPT ); Wed, 15 Apr 2015 23:52:14 -0400 Date: Wed, 15 Apr 2015 22:51:57 -0500 From: David Fries To: Jonathan ALIBERT Cc: Evgeniy Polyakov , Thorsten Bschorr , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] Avoid null-pointer access in w1/slaves/w1_therm Message-ID: <20150416035157.GI3409@spacedout.fries.net> References: <20150308211449.GG11991@spacedout.fries.net> <20150309230924.GJ11991@spacedout.fries.net> <2663771425995520@web30h.yandex.ru> <20150312005408.GD23778@spacedout.fries.net> <2283421426366516@web2o.yandex.ru> <20150318042037.GE21067@spacedout.fries.net> <702441426691933@web2g.yandex.ru> <20150319000908.GG21067@spacedout.fries.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.5.11 (SpacedOut.fries.net [127.0.0.1]); Wed, 15 Apr 2015 22:52:01 -0500 (CDT) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2584 Lines: 71 It has not been solved. Evgeniy would like to make use of the sysfs device management instead of the current reference counting, however I haven't heard any volunteers to do that work. I posted a quick fix patch, it was very easy to crash without this patch, it doesn't completely solve the race conditions, and I don't think it can be solved in just a slave driver change. Are you up for the challenge? On Wed, Apr 15, 2015 at 09:52:27AM +0200, Jonathan ALIBERT wrote: > Hi, > > Do you know if the problem has been solved ? > > Cheers, > > *Jonathan ALIBERT* > *06 32 26 59 12* > *265, route de Saint Haon* > *42 370 RENAISON* > > > 2015-03-19 1:09 GMT+01:00 David Fries : > > > On Wed, Mar 18, 2015 at 06:18:53PM +0300, Evgeniy Polyakov wrote: > > > Hi > > > > > > 18.03.2015, 07:20, "David Fries" : > > > > static void w1_therm_remove_slave(struct w1_slave *sl) > > > > { > > > > + int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data)); > > > > + while(refcnt) { > > > > + msleep(1000); > > > > + refcnt = atomic_read(THERM_REFCNT(sl->family_data)); > > > > + } > > > > kfree(sl->family_data); > > > > sl->family_data = NULL; > > > > } > > > > > > Can we replace this whole atomic manipulations with kref_t and free > > family data in the place > > > which actually drops reference counter to zero? > > > > > > I.e. we return from remove_slave() function potentially leaving family > > data floating around, it will be freed > > > when the last user drops the reference. There is still a race between > > increasing reference when starting > > > reading and removing slave device, i.e. one starts reading, while > > attached slave device is being removed, > > > but that's a different problem. > > > > With the two while loops I posted, I see with two clients reading > > w1_slave, the other command to remove a slave gets permanently stuck > > in w1_therm_remove_slave, which keeps the slave around while the > > clients continue to read. I wouldn't predict things going better by > > keeping family_data around longer, the slave data would still go away > > with readers around. > > > > -- > > David Fries PGP pub CB1EE8F0 > > http://fries.net/~david/ > > -- David Fries PGP pub CB1EE8F0 http://fries.net/~david/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/