From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pv0-f174.google.com ([74.125.83.174]:44448 "EHLO mail-pv0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932125Ab0LMOEn (ORCPT ); Mon, 13 Dec 2010 09:04:43 -0500 Date: Mon, 13 Dec 2010 22:04:21 +0800 From: Dave Young To: Torsten Kaiser Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Guennadi Liakhovetski , Chris Clayton Subject: Re: [PATCH] bttv: fix mutex use before init Message-ID: <20101213140421.GA2826@darkstar> References: <20101212131550.GA2608@darkstar> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: List-ID: Sender: Mauro Carvalho Chehab On Sun, Dec 12, 2010 at 05:13:47PM +0100, Torsten Kaiser wrote: > On Sun, Dec 12, 2010 at 2:15 PM, Dave Young wrote: > > oops happen in bttv_open while locking uninitialized mutex fh->cap.vb_lock > > add mutex_init before usage > > I have seen the same problem twice since I switched of the BKL in > 2.6.37-rc2, but only had the time today to search for the cause. > (It only happend on two boots out of probably more then 50, so I only > investigated after it happened a second time with -rc5) > > The cause was making this change to bttv_open() and radio_open() to > remove the BKL from them: > + mutex_lock(&fh->cap.vb_lock); > *fh = btv->init; > + mutex_unlock(&fh->cap.vb_lock); > > This is wrong because of two things: > First: The fh->cap.vb_lock has not been initialised, as you noted. > Second: The middle line will overwrite the mutex! > > Just adding mutex_init() will not really help the second problem and > seems to be wrong from the point that cap.vb_lock already has a > mutex_init() via videobuf_queue_sg_init(). Yes, you are right. I wonder if the above lock code in bttv_open and radio_open can be removed or just use btv->lock. > > I'm not sure what the correct fix is, but I would rather suggest this: > > * change &fh->cap.vb_lock in bttv_open() AND radio_open() to > &btv->init.cap.vb_lock > * add a mutex_init(&btv->init.cap.vb_lock) to the setup of init in bttv_probe() I will test this when the bttv card is available for me tommorrow. > > > Signed-off-by: Dave Young > > Tested-by: Chris Clayton > > --- > >  drivers/media/video/bt8xx/bttv-driver.c |    2 ++ > >  1 file changed, 2 insertions(+) > > > > --- linux-2.6.orig/drivers/media/video/bt8xx/bttv-driver.c      2010-11-27 11:21:30.000000000 +0800 > > +++ linux-2.6/drivers/media/video/bt8xx/bttv-driver.c   2010-12-12 16:31:39.633333338 +0800 > > @@ -3291,6 +3291,8 @@ static int bttv_open(struct file *file) > >        fh = kmalloc(sizeof(*fh), GFP_KERNEL); > >        if (unlikely(!fh)) > >                return -ENOMEM; > > + > > +       mutex_init(&fh->cap.vb_lock); > >        file->private_data = fh; > > > >        /* > > -- > > 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/ > >