linux-kernel - Re: [HACK] convert i_alloc_sem for direct

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: <20070925144257.1acedd24@twins>
Date:	Tue, 25 Sep 2007 14:42:57 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	linux-rt-users <linux-rt-users@...r.kernel.org>,
	mingo@...dmis.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [HACK] convert i_alloc_sem for direct_io.c craziness!

On Mon, 24 Sep 2007 17:14:26 -0400 (EDT) Steven Rostedt
<rostedt@...dmis.org> wrote:

> 
> Hopefully I will get some attention from those that are responsible for
> fs/direct_io.c
> 
> Ingo and Thomas,
> 
> This patch converts the i_alloc_sem into a compat_rw_semaphore for the -rt
> patch.  Seems that the code in fs/direct_io.c does some nasty logic with
> the i_alloc_sem.  For DIO_LOCKING, I'm assuming that the i_alloc_sem is
> used as a reference counter for pending requests. When the request is
> made, the down_read is performed. When the request is handled by the block
> softirq, then that softirq does an up on the request. So the owner is not
> the same between down and up. When all requests are handled, the semaphore
> counter should be zero. This keeps away any write access while requests
> are pending.
> 
> Now this may all be well and dandy for vanilla Linux, but it breaks
> miserbly when converted to -rt.
> 
> 1) In RT rw_semaphores must be up'd by the same thread that down's it.
> 
> 2) We can't do PI on the correct processes.
> 
> This patch converts (for now) the i_alloc_sem into a compat_rw_semaphore
> to give back the old features to the sem. This fixes deadlocks that we've
> been having WRT direct_io.  But unfortunately, it now opens up
> unbonded priority inversion with this semaphore.  But really, those that
> can be affected by this, shouldn't be doing disk IO anyway.
> 
> The real fix would be to get rid of the read semaphore trickery in
> direct_io.c.

How about teaching {up,down}_read_non_owner() to barf on rw_semaphore
in -rt?

> Signed-off-by: Steve Rostedt <rostedt@...dmis.org>
> 
> Index: linux-2.6.23-rc4-rt1/include/linux/fs.h
> ===================================================================
> --- linux-2.6.23-rc4-rt1.orig/include/linux/fs.h	2007-09-24 16:58:59.000000000 -0400
> +++ linux-2.6.23-rc4-rt1/include/linux/fs.h	2007-09-24 16:59:11.000000000 -0400
> @@ -577,7 +577,7 @@ struct inode {
>  	umode_t			i_mode;
>  	spinlock_t		i_lock;	/* i_blocks, i_bytes, maybe i_size */
>  	struct mutex		i_mutex;
> -	struct rw_semaphore	i_alloc_sem;
> +	struct compat_rw_semaphore	i_alloc_sem;
>  	const struct inode_operations	*i_op;
>  	const struct file_operations	*i_fop;	/* former ->i_op->default_file_ops */
>  	struct super_block	*i_sb;
> -
> 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/
-
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/