linux-kernel - Re: [PATCH] raw1394: Push the BKL down into the driver ioctls

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: <20080523115741.5d695706@core>
Date:	Fri, 23 May 2008 11:57:41 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Stefan Richter <stefanr@...6.in-berlin.de>
Cc:	linux-kernel@...r.kernel.org, linux1394-devel@...ts.sourceforge.net
Subject: Re: [PATCH] raw1394: Push the BKL down into the driver ioctls

On Fri, 23 May 2008 12:23:09 +0200
Stefan Richter <stefanr@...6.in-berlin.de> wrote:

> Alan Cox wrote:
> > Actually in this case wrap the function for now.
> > 
> > Signed-off-by: Alan Cox <alan@...hat.com>
> 
> Can an .unlocked_ioctl() preempt another .unlocked_ioctl() to the very
> same instance of struct file?

Yes. And this btw is true even with the old locked ioctl call if you ever
sleep (eg a copy_to/from_user).

> If yes, we need to serialize do_raw1394_ioctl against itself or come up
> with another protection against concurrent fi->iso_state switches before
> we can remove lock_kernel().  And if a .write() can preempt another
> .write() to the same instance of struct file, raw1394_write() already
> has a problem with concurrent fi->state switches.

Quite a few drivers end up with a private mutex and do mutex_lock/unlock
around the ioctl and write paths (and if the write path can be slow using
_trylock and O_NDELAY check when appropriate).

The goal of pushing it down is to enable driver authors to see and to do
the locking at a driver level instead - plus fix lots of bugs where the
BKL "sleep and drop" behaviour isn't anticipated.

> The same s/raw1394_ioctl/do_raw1394_ioctl/ should be done in
> raw1394_compat_ioctl().  But I suppose it doesn't really matter because
> lock_kernel() is allowed to nest.

Agreed.

Alan
--
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/