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  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, 21 Mar 2014 13:00:55 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Fabian Frederick <fabf@...net.be>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	reiserfs-devel@...r.kernel.org
Subject: Re: [RFC 1/1] fs/reiserfs/journal.c: Remove obsolete  __GFP_NOFAIL

On Fri, 21 Mar 2014 17:18:30 +0100 Fabian Frederick <fabf@...net.be> wrote:

> Loop around congestion_wait on allocation failure/alloc_journal_list
> like already fixed in other FS.
> 
> ...
>
> --- a/fs/reiserfs/journal.c
> +++ b/fs/reiserfs/journal.c
> @@ -2487,8 +2487,13 @@ static int journal_read(struct super_block *sb)
>  static struct reiserfs_journal_list *alloc_journal_list(struct super_block *s)
>  {
>  	struct reiserfs_journal_list *jl;
> -	jl = kzalloc(sizeof(struct reiserfs_journal_list),
> -		     GFP_NOFS | __GFP_NOFAIL);
> +
> +	do {
> +		jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
> +		if (unlikely(!jl))
> +			congestion_wait(BLK_RW_ASYNC, HZ/50);
> +	} while (!jl)
> +

Dammit, who has been running around converting __GFP_NOFAIL into
open-coded congestion_wait() loops?

The whole point of __GFP_NOFAIL is to centralise this
wait-for-memory-for-ever operation.  So it is implemented in a common
(core) place and so that we can easily locate these problematic
callers.

This comment in ext4:

			/*
			 * If __GFP_FS is not present, then we may be
			 * being called from inside the fs writeback
			 * layer, so we MUST NOT fail.  Since
			 * __GFP_NOFAIL is going away, we will arrange
			 * to retry the allocation ourselves.
			 */

is exactly wrong.  Yes, we'd like __GFP_NOFAIL to go away, but it
cannot go away until buggy callsites such as this one are *fixed*. 
Removing the __GFP_NOFAIL usage simply hides the buggy code from casual
searchers.

argh.

What we should do is to fix all these call sites so they can handle
memory exhaustion.  That's hard so in the interim they should be using
__GFP_NOFAIL.

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

Powered by blists - more mailing lists