linux-kernel - Re: [PATCH] mm: check memory reclaim bugs caused fs reentrance

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] [day] [month] [year] [list]

Message-ID: <87fvdox0b8.fsf@openvz.org>
Date:	Wed, 12 Nov 2014 11:58:19 +0300
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	Dave Chinner <david@...morbit.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-btrfs@...r.kernel.org
Subject: Re: [PATCH] mm: check memory reclaim bugs caused fs reentrance

Dave Chinner <david@...morbit.com> writes:

> On Tue, Nov 11, 2014 at 03:49:50PM +0400, Dmitry Monakhov wrote:
>> If filesystem holds transaction open 'current->journal_info' it should not
>> performs memory allocations with __GFP_FS flag enabled otherwise this result in fs
>> reentarance which lead to:
>> 1) reentrance to itself : deadlock or internal assertion failure due to incorrect journal credits
>> 1) entrance to another fs: assertion faulure or silient corruption due to incorrect journal
>> 
>> Signed-off-by: Dmitry Monakhov <dmonakhov@...nvz.org>
>> ---
>>  include/linux/kernel.h |    7 +++++++
>>  mm/dmapool.c           |    1 +
>>  mm/mempool.c           |    1 +
>>  mm/page_alloc.c        |    1 +
>>  mm/slab.c              |    1 +
>>  mm/slub.c              |    1 +
>>  6 files changed, 12 insertions(+), 0 deletions(-)
>> 
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index 3d770f5..69923d4 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -232,6 +232,13 @@ void might_fault(void);
>>  static inline void might_fault(void) { }
>>  #endif
>>  
>> +#ifdef CONFIG_PROVE_LOCKING
>> +#define might_enter_fs_if(cond) \
>> +	 WARN_ON_ONCE((cond) && current->journal_info)
>
> XFS does not use current->journal_info, and so this won't ever
> trigger on XFS. XFS uses PF_FSTRANS to indicate a transaction is in
> progress.
Yes, I've simply forget about that.
>
> Besides, isn't this redundant functionality? Lockdep already catches
> these problems with it's reclaim context tracking and it's tracking
> is more extensive than this simple check like this.  lockdep
> regularly pointed out allocation/reclaim context problems in XFS
> until we fixed them....
This is correct, but only partly, at this moment lockdep are not very good at
catching fs re-entrance. It has FS_RECLAIM machinery but it is not idial.
So I'll rewrite my patch and add explicit fs re-entrance rule to lockdep
>
> Cheers,
>
> Dave.
> -- 
> Dave Chinner
> david@...morbit.com

Download attachment "signature.asc" of type "application/pgp-signature" (473 bytes)