All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Anthony Romano <anthony.romano@coreos.com>, hughd@google.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] tmpfs: don't undo fallocate past its last page
Date: Mon, 16 May 2016 13:59:10 +0200	[thread overview]
Message-ID: <5739B60E.1090700@suse.cz> (raw)
In-Reply-To: <1462713387-16724-1-git-send-email-anthony.romano@coreos.com>

On 05/08/2016 03:16 PM, Anthony Romano wrote:
> When fallocate is interrupted it will undo a range that extends one byte
> past its range of allocated pages. This can corrupt an in-use page by
> zeroing out its first byte. Instead, undo using the inclusive byte range.

Huh, good catch. So why is shmem_undo_range() adding +1 to the value in 
the first place? The only other caller is shmem_truncate_range() and all 
*its* callers do subtract 1 to avoid the same issue. So a nicer fix 
would be to remove all this +1/-1 madness. Or is there some subtle 
corner case I'm missing?

> Signed-off-by: Anthony Romano <anthony.romano@coreos.com>

Looks like a stable candidate patch. Can you point out the commit that 
introduced the bug, for the Fixes: tag?

Thanks,
Vlastimil

> ---
>   mm/shmem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 719bd6b..f0f9405 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2238,7 +2238,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
>   			/* Remove the !PageUptodate pages we added */
>   			shmem_undo_range(inode,
>   				(loff_t)start << PAGE_SHIFT,
> -				(loff_t)index << PAGE_SHIFT, true);
> +				((loff_t)index << PAGE_SHIFT) - 1, true);
>   			goto undone;
>   		}
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Anthony Romano <anthony.romano@coreos.com>, hughd@google.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] tmpfs: don't undo fallocate past its last page
Date: Mon, 16 May 2016 13:59:10 +0200	[thread overview]
Message-ID: <5739B60E.1090700@suse.cz> (raw)
In-Reply-To: <1462713387-16724-1-git-send-email-anthony.romano@coreos.com>

On 05/08/2016 03:16 PM, Anthony Romano wrote:
> When fallocate is interrupted it will undo a range that extends one byte
> past its range of allocated pages. This can corrupt an in-use page by
> zeroing out its first byte. Instead, undo using the inclusive byte range.

Huh, good catch. So why is shmem_undo_range() adding +1 to the value in 
the first place? The only other caller is shmem_truncate_range() and all 
*its* callers do subtract 1 to avoid the same issue. So a nicer fix 
would be to remove all this +1/-1 madness. Or is there some subtle 
corner case I'm missing?

> Signed-off-by: Anthony Romano <anthony.romano@coreos.com>

Looks like a stable candidate patch. Can you point out the commit that 
introduced the bug, for the Fixes: tag?

Thanks,
Vlastimil

> ---
>   mm/shmem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 719bd6b..f0f9405 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2238,7 +2238,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
>   			/* Remove the !PageUptodate pages we added */
>   			shmem_undo_range(inode,
>   				(loff_t)start << PAGE_SHIFT,
> -				(loff_t)index << PAGE_SHIFT, true);
> +				((loff_t)index << PAGE_SHIFT) - 1, true);
>   			goto undone;
>   		}
>
>

  reply	other threads:[~2016-05-16 11:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-08 13:16 [PATCH] tmpfs: don't undo fallocate past its last page Anthony Romano
2016-05-08 13:16 ` Anthony Romano
2016-05-16 11:59 ` Vlastimil Babka [this message]
2016-05-16 11:59   ` Vlastimil Babka
2016-05-16 13:55   ` Anthony Romano
2016-06-04  1:10   ` Brandon Philips
2016-06-06  4:05   ` Brandon Philips
2016-06-06  4:05     ` Brandon Philips

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5739B60E.1090700@suse.cz \
    --to=vbabka@suse.cz \
    --cc=anthony.romano@coreos.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.