ACK/CMNT: [PATCH 2/2] Squashfs: Compute expected length from inode size rather than block length
Tyler Hicks
tyhicks at canonical.com
Wed Feb 20 13:35:07 UTC 2019
On 2019-02-19 15:32:31, Paolo Pisati wrote:
> From: Phillip Lougher <phillip at squashfs.org.uk>
This line should be added when committing this change:
BugLink: https://bugs.launchpad.net/bugs/1816756
With that change,
Acked-by: Tyler Hicks <tyhicks at canonical.com>
Tyler
>
> Previously in squashfs_readpage() when copying data into the page
> cache, it used the length of the datablock read from the filesystem
> (after decompression). However, if the filesystem has been corrupted
> this data block may be short, which will leave pages unfilled.
>
> The fix for this is to compute the expected number of bytes to copy
> from the inode size, and use this to detect if the block is short.
>
> Signed-off-by: Phillip Lougher <phillip at squashfs.org.uk>
> Tested-by: Willy Tarreau <w at 1wt.eu>
> Cc: Анатолий Тросиненко <anatoly.trosinenko at gmail.com>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> (cherry picked from commit a3f94cb99a854fa381fe7fadd97c4f61633717a5)
> Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> ---
> fs/squashfs/file.c | 25 ++++++++++---------------
> fs/squashfs/file_cache.c | 4 ++--
> fs/squashfs/file_direct.c | 16 +++++++++++-----
> fs/squashfs/squashfs.h | 2 +-
> 4 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index fa3eb0a..68a29c9 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -431,10 +431,9 @@ skip_page:
> }
>
> /* Read datablock stored packed inside a fragment (tail-end packed block) */
> -static int squashfs_readpage_fragment(struct page *page)
> +static int squashfs_readpage_fragment(struct page *page, int expected)
> {
> struct inode *inode = page->mapping->host;
> - struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
> struct squashfs_cache_entry *buffer = squashfs_get_fragment(inode->i_sb,
> squashfs_i(inode)->fragment_block,
> squashfs_i(inode)->fragment_size);
> @@ -445,23 +444,16 @@ static int squashfs_readpage_fragment(struct page *page)
> squashfs_i(inode)->fragment_block,
> squashfs_i(inode)->fragment_size);
> else
> - squashfs_copy_cache(page, buffer, i_size_read(inode) &
> - (msblk->block_size - 1),
> + squashfs_copy_cache(page, buffer, expected,
> squashfs_i(inode)->fragment_offset);
>
> squashfs_cache_put(buffer);
> return res;
> }
>
> -static int squashfs_readpage_sparse(struct page *page, int index, int file_end)
> +static int squashfs_readpage_sparse(struct page *page, int expected)
> {
> - struct inode *inode = page->mapping->host;
> - struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
> - int bytes = index == file_end ?
> - (i_size_read(inode) & (msblk->block_size - 1)) :
> - msblk->block_size;
> -
> - squashfs_copy_cache(page, NULL, bytes, 0);
> + squashfs_copy_cache(page, NULL, expected, 0);
> return 0;
> }
>
> @@ -471,6 +463,9 @@ static int squashfs_readpage(struct file *file, struct page *page)
> struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
> int index = page->index >> (msblk->block_log - PAGE_CACHE_SHIFT);
> int file_end = i_size_read(inode) >> msblk->block_log;
> + int expected = index == file_end ?
> + (i_size_read(inode) & (msblk->block_size - 1)) :
> + msblk->block_size;
> int res;
> void *pageaddr;
>
> @@ -489,11 +484,11 @@ static int squashfs_readpage(struct file *file, struct page *page)
> goto error_out;
>
> if (bsize == 0)
> - res = squashfs_readpage_sparse(page, index, file_end);
> + res = squashfs_readpage_sparse(page, expected);
> else
> - res = squashfs_readpage_block(page, block, bsize);
> + res = squashfs_readpage_block(page, block, bsize, expected);
> } else
> - res = squashfs_readpage_fragment(page);
> + res = squashfs_readpage_fragment(page, expected);
>
> if (!res)
> return 0;
> diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
> index f2310d2..a9ba8d9 100644
> --- a/fs/squashfs/file_cache.c
> +++ b/fs/squashfs/file_cache.c
> @@ -20,7 +20,7 @@
> #include "squashfs.h"
>
> /* Read separately compressed datablock and memcopy into page cache */
> -int squashfs_readpage_block(struct page *page, u64 block, int bsize)
> +int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expected)
> {
> struct inode *i = page->mapping->host;
> struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
> @@ -31,7 +31,7 @@ int squashfs_readpage_block(struct page *page, u64 block, int bsize)
> ERROR("Unable to read page, block %llx, size %x\n", block,
> bsize);
> else
> - squashfs_copy_cache(page, buffer, buffer->length, 0);
> + squashfs_copy_cache(page, buffer, expected, 0);
>
> squashfs_cache_put(buffer);
> return res;
> diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
> index f4fcb8c..21ccc4b 100644
> --- a/fs/squashfs/file_direct.c
> +++ b/fs/squashfs/file_direct.c
> @@ -21,10 +21,11 @@
> #include "page_actor.h"
>
> static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
> - int pages, struct page **page);
> + int pages, struct page **page, int bytes);
>
> /* Read separately compressed datablock directly into page cache */
> -int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
> +int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
> + int expected)
>
> {
> struct inode *inode = target_page->mapping->host;
> @@ -83,7 +84,7 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
> * using an intermediate buffer.
> */
> res = squashfs_read_cache(target_page, block, bsize, pages,
> - page);
> + page, expected);
> if (res < 0)
> goto mark_errored;
>
> @@ -95,6 +96,11 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize)
> if (res < 0)
> goto mark_errored;
>
> + if (res != expected) {
> + res = -EIO;
> + goto mark_errored;
> + }
> +
> /* Last page may have trailing bytes not filled */
> bytes = res % PAGE_CACHE_SIZE;
> if (bytes) {
> @@ -138,12 +144,12 @@ out:
>
>
> static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
> - int pages, struct page **page)
> + int pages, struct page **page, int bytes)
> {
> struct inode *i = target_page->mapping->host;
> struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
> block, bsize);
> - int bytes = buffer->length, res = buffer->error, n, offset = 0;
> + int res = buffer->error, n, offset = 0;
>
> if (res) {
> ERROR("Unable to read page, block %llx, size %x\n", block,
> diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
> index d8d4372..f89f8a7 100644
> --- a/fs/squashfs/squashfs.h
> +++ b/fs/squashfs/squashfs.h
> @@ -72,7 +72,7 @@ void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
> int);
>
> /* file_xxx.c */
> -extern int squashfs_readpage_block(struct page *, u64, int);
> +extern int squashfs_readpage_block(struct page *, u64, int, int);
>
> /* id.c */
> extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list