From owner-freebsd-hackers@FreeBSD.ORG Mon Feb 3 20:28:47 2014 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C4118820 for ; Mon, 3 Feb 2014 20:28:47 +0000 (UTC) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 969C210DE for ; Mon, 3 Feb 2014 20:28:47 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1WAQ8M-0004h5-53; Mon, 03 Feb 2014 20:28:46 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id s13KShAA063467; Mon, 3 Feb 2014 13:28:43 -0700 (MST) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 24.8.230.52 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1+GsyLnOmLD9DYq7EFQ9kQj Subject: Re: Races in ichsmb(9) when accessed from a multithreaded process From: Ian Lepore To: Ryan Stone In-Reply-To: References: Content-Type: text/plain; charset="us-ascii" Date: Mon, 03 Feb 2014 13:28:43 -0700 Message-ID: <1391459323.13026.100.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: "freebsd-hackers@freebsd.org" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Feb 2014 20:28:47 -0000 On Mon, 2014-02-03 at 11:41 -0500, Ryan Stone wrote: > ichsmb.c has the following rather worrisome comment: > > * This driver assumes that the generic SMBus code will ensure that > * at most one process at a time calls into the SMBus methods below. > > However, when I look at the code is sys/dev/smbus, I see nothing that > actually guarantees this if two threads in the same process call > ioctls on the same file descriptor. It does call smbus_request_bus, > but mostly that just calls down into the smbus implementation (in this > case ichsmb) with SMBUS_CALLBACK. ichsmb always just acks the > request, so no actual locking ends up occurring. > > Is it intended that smb(9) clients be required to do their own > locking? It seems to me that that is way more fragile than it needs > to be. It looks to me like most of the SMBUS_CALLBACK implementations are stubbed out, and thus are wrong. The notable exceptions are iicbus/iicsmb.c which relies on the underlying iic driver to get it right (and I know at least some of them do), and bktr/bktr_i2c.c which has what looks like good exclusive-bus-grant logic (modulo the use of Giant for locking). -- Ian