Date: Mon, 03 Feb 2014 13:28:43 -0700 From: Ian Lepore <ian@FreeBSD.org> To: Ryan Stone <rysto32@gmail.com> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@FreeBSD.org> Subject: Re: Races in ichsmb(9) when accessed from a multithreaded process Message-ID: <1391459323.13026.100.camel@revolution.hippie.lan> In-Reply-To: <CAFMmRNxLWcxV2dYdr8R9ihrSgy4Fma9YELBmResU2KrQEQd6XA@mail.gmail.com> References: <CAFMmRNxLWcxV2dYdr8R9ihrSgy4Fma9YELBmResU2KrQEQd6XA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1391459323.13026.100.camel>