linux-kernel - Re: [PATCH] mm: fix cpu hangs on truncating last page of a 16t sparse file

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:	Sat, 26 Sep 2015 18:36:29 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	angelo <angelo70@...il.com>
cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH] mm: fix cpu hangs on truncating last page of a 16t sparse
 file

Let's Cc linux-fsdevel, who will be more knowledgable.

On Sun, 27 Sep 2015, angelo wrote:

> Hi all,
> 
> running xfstests, generic 308 on whatever 32bit arch is possible
> to observe cpu to hang near 100% on unlink.
> The test removes a sparse file of length 16tera where only the last
> 4096 bytes block is mapped.
> At line 265 of truncate.c there is a
> if (index >= end)
>     break;
> But if index is, as in this case, a 4294967295, it match -1 used as
> eof. Hence the cpu loops 100% just after.

That's odd.  I've not checked your patch, because I think the problem
would go beyond truncate, and the root cause lie elsewhere.

My understanding is that the 32-bit
#define MAX_LFS_FILESIZE (((loff_t)PAGE_CACHE_SIZE << (BITS_PER_LONG-1))-1) 
makes a page->index of -1 (or any "negative") impossible to reach.

I don't know offhand the rules for mounting a filesystem populated with
a 64-bit kernel on a 32-bit kernel, what's to happen when a too-large
file is encountered; but assume that's not the case here - you're
just running xfstests/tests/generic/308.

Is pwrite missing a check for offset beyond s_maxbytes?

Or is this filesystem-dependent?  Which filesystem?

Hugh

> 
> -------------------
> 
> On 32bit archs, with CONFIG_LBDAF=y, if truncating last page
> of a 16tera file, "index" variable is set to 4294967295, and hence
> matches with -1 used as EOF value. This result in an inifite loop
> when unlink is executed on this file.
> 
> Signed-off-by: Angelo Dureghello <angelo@...am.it>
> ---
>  mm/truncate.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 76e35ad..3751034 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -283,14 +283,15 @@ void truncate_inode_pages_range(struct address_space
> *mapping,
>                 pagevec_remove_exceptionals(&pvec);
>                 pagevec_release(&pvec);
>                 cond_resched();
> -               index++;
> +               if (index < end)
> +                       index++;
>         }
> 
>         if (partial_start) {
>                 struct page *page = find_lock_page(mapping, start - 1);
>                 if (page) {
>                         unsigned int top = PAGE_CACHE_SIZE;
> -                       if (start > end) {
> +                       if (start > end && end != -1) {
>                                 /* Truncation within a single page */
>                                 top = partial_end;
>                                 partial_end = 0;
> @@ -322,7 +323,7 @@ void truncate_inode_pages_range(struct address_space
> *mapping,
>          * If the truncation happened within a single page no pages
>          * will be released, just zeroed, so we can bail out now.
>          */
> -       if (start >= end)
> +       if (start >= end && end != -1)
>                 return;
> 
>         index = start;
> @@ -337,7 +338,7 @@ void truncate_inode_pages_range(struct address_space
> *mapping,
>                         index = start;
>                         continue;
>                 }
> -               if (index == start && indices[0] >= end) {
> +               if (index == start && (indices[0] >= end && end != -1)) {
>                         /* All gone out of hole to be punched, we're done */
>                         pagevec_remove_exceptionals(&pvec);
>                         pagevec_release(&pvec);
> @@ -348,7 +349,7 @@ void truncate_inode_pages_range(struct address_space
> *mapping,
> 
>                         /* We rely upon deletion not changing page->index */
>                         index = indices[i];
> -                       if (index >= end) {
> +                       if (index >= end && (end != -1)) {
>                                 /* Restart punch to make sure all gone */
>                                 index = start - 1;
>                                 break;
> -- 
> 2.5.3
--
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/