linux-kernel - Re: [PATCH 08/18] lirc driver for the Soundgraph IMON IR Receivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200809241621.38715.jarod@redhat.com>
Date:	Wed, 24 Sep 2008 16:21:38 -0400
From:	Jarod Wilson <jarod@...hat.com>
To:	Jonathan Corbet <corbet@....net>
Cc:	linux-kernel@...r.kernel.org, Janne Grunau <j@...nau.net>,
	Christoph Bartelmus <lirc@...telmus.de>
Subject: Re: [PATCH 08/18] lirc driver for the Soundgraph IMON IR Receivers

On Monday 22 September 2008 17:47:56 Jarod Wilson wrote:
> > > +/**
> > > + * Called by lirc_dev when the application opens /dev/lirc
> > > + */
> > > +static int ir_open(void *data)
> > > +{
> > > +   int retval = SUCCESS;
> > > +   struct imon_context *context;
> > > +
> > > +   /* prevent races with disconnect */
> > > +   down(&disconnect_sem);
> > > +
> > > +   context = (struct imon_context *) data;
> > > +
> > > +   LOCK_CONTEXT;
> > > +
> > > +   if (context->ir_isopen) {
> > > +           err("%s: IR port is already open", __func__);
> > > +           retval = -EBUSY;
> > > +           goto exit;
> > > +   }
> >
> > I wonder if the single-open semantics are really doing what the author
> > intended?  It is unsufficient to prevent concurrent calls elsewhere.
>
> I think we're good to go with disconnect_sem being converted to an actual
> mutex, no?

No. The lock here is worthless, as is the check for context->ir_isopen, as 
this function *only* gets called with the driver_lock held in lirc_dev, and 
lirc_dev won't call this if the device is already open. I've simply dropped 
the context lock, as its not needed at all (and in fact, triggered lockdep 
spew).

-- 
Jarod Wilson
jarod@...hat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/