linux-kernel - Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")

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]

Date:	Fri, 16 May 2008 18:31:29 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression
 caused by bf726e "semaphore: fix,")

On Sat, 17 May 2008, OGAWA Hirofumi wrote:
> >  static struct inode *fat_alloc_inode(struct super_block *sb)
> >  {
> >  	struct msdos_inode_info *ei;
> > -	ei = kmem_cache_alloc(fat_inode_cachep, GFP_KERNEL);
> > +	ei = kmem_cache_alloc(fat_inode_cachep, GFP_NOFS);
> >  	if (!ei)
> >  		return NULL;
> >  	return &ei->vfs_inode;
> 
> Um... do we need this? I think this path is not called from memory
> allocation path...

The issue isn't that *this* is called by memory allocation paths, but 
whether this can hold the lock and then some memory allocation path calls 
back to the filesystem two write something out - and deadlock.

In other words, the chain is something like this:

	msdos_lookup
	  lock_super()			****
	  fat_build_inode ->
	    new_inode() ->
	      s->s_ops->alloc_inode = fat_alloc_inode ->
	        kmem_cache_alloc() ->
	          .. pageout ..
	            sync_inodes ->
	            (or any other writeout)
	              lock_super()	****

and now it deadlocks.

In other words, the kmem_cache_alloc() must _not_ be allowed to actually 
cause a filesystem writeout, and that's what GFP_NOFS is all about.

That said, just removing the lock-kernel entirely obviously would not have 
that problem either, but I feel safer at least keeping the same locking it 
had, rather than removing locking entirely.

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