ACK/CMNT: [PATCH 1/2] squashfs metadata 2: electric boogaloo

[Tyler Hicks]

On 2019-02-19 15:32:30, Paolo Pisati wrote:
> From: Linus Torvalds <torvalds at linux-foundation.org>

This line should be added when committing this change:

BugLink: https://bugs.launchpad.net/bugs/1816756

> 
> Anatoly continues to find issues with fuzzed squashfs images.
> 
> This time, corrupt, missing, or undersized data for the page filling
> wasn't checked for, because the squashfs_{copy,read}_cache() functions
> did the squashfs_copy_data() call without checking the resulting data
> size.
> 
> Which could result in the page cache pages being incompletely filled in,
> and no error indication to the user space reading garbage data.
> 
> So make a helper function for the "fill in pages" case, because the
> exact same incomplete sequence existed in two places.
> 
> [ I should have made a squashfs branch for these things, but I didn't
>   intend to start doing them in the first place.
> 
>   My historical connection through cramfs is why I got into looking at
>   these issues at all, and every time I (continue to) think it's a
>   one-off.
> 
>   Because _this_ time is always the last time. Right?   - Linus ]
> 
> Reported-by: Anatoly Trosinenko <anatoly.trosinenko at gmail.com>
> Tested-by: Willy Tarreau <w at 1wt.eu>
> Cc: Al Viro <viro at zeniv.linux.org.uk>
> Cc: Phillip Lougher <phillip at squashfs.org.uk>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> (cherry picked from commit cdbb65c4c7ead680ebe54f4f0d486e2847a500ea)

You had to change PAGE_SIZE to PAGE_CACHE_SIZE so the above line needs
s/cherry picked/backported/

With the above two changes,

Acked-by: Tyler Hicks <tyhicks at canonical.com>

Tyler

> Signed-off-by: Paolo Pisati <paolo.pisati at canonical.com>
> ---
>  fs/squashfs/file.c        | 25 ++++++++++++++++++-------
>  fs/squashfs/file_direct.c |  8 +-------
>  fs/squashfs/squashfs.h    |  1 +
>  3 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 1ec7bae2..fa3eb0a 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -374,13 +374,29 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
>  	return squashfs_block_size(size);
>  }
>  
> +void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer, int offset, int avail)
> +{
> +	int copied;
> +	void *pageaddr;
> +
> +	pageaddr = kmap_atomic(page);
> +	copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
> +	memset(pageaddr + copied, 0, PAGE_CACHE_SIZE - copied);
> +	kunmap_atomic(pageaddr);
> +
> +	flush_dcache_page(page);
> +	if (copied == avail)
> +		SetPageUptodate(page);
> +	else
> +		SetPageError(page);
> +}
> +
>  /* Copy data into page cache  */
>  void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
>  	int bytes, int offset)
>  {
>  	struct inode *inode = page->mapping->host;
>  	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
> -	void *pageaddr;
>  	int i, mask = (1 << (msblk->block_log - PAGE_CACHE_SHIFT)) - 1;
>  	int start_index = page->index & ~mask, end_index = start_index | mask;
>  
> @@ -406,12 +422,7 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
>  		if (PageUptodate(push_page))
>  			goto skip_page;
>  
> -		pageaddr = kmap_atomic(push_page);
> -		squashfs_copy_data(pageaddr, buffer, offset, avail);
> -		memset(pageaddr + avail, 0, PAGE_CACHE_SIZE - avail);
> -		kunmap_atomic(pageaddr);
> -		flush_dcache_page(push_page);
> -		SetPageUptodate(push_page);
> +		squashfs_fill_page(push_page, buffer, offset, avail);
>  skip_page:
>  		unlock_page(push_page);
>  		if (i != page->index)
> diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
> index 43e7a7e..f4fcb8c 100644
> --- a/fs/squashfs/file_direct.c
> +++ b/fs/squashfs/file_direct.c
> @@ -144,7 +144,6 @@ static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
>  	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
>  						 block, bsize);
>  	int bytes = buffer->length, res = buffer->error, n, offset = 0;
> -	void *pageaddr;
>  
>  	if (res) {
>  		ERROR("Unable to read page, block %llx, size %x\n", block,
> @@ -159,12 +158,7 @@ static int squashfs_read_cache(struct page *target_page, u64 block, int bsize,
>  		if (page[n] == NULL)
>  			continue;
>  
> -		pageaddr = kmap_atomic(page[n]);
> -		squashfs_copy_data(pageaddr, buffer, offset, avail);
> -		memset(pageaddr + avail, 0, PAGE_CACHE_SIZE - avail);
> -		kunmap_atomic(pageaddr);
> -		flush_dcache_page(page[n]);
> -		SetPageUptodate(page[n]);
> +		squashfs_fill_page(page[n], buffer, offset, avail);
>  		unlock_page(page[n]);
>  		if (page[n] != target_page)
>  			page_cache_release(page[n]);
> diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
> index 887d6d2..d8d4372 100644
> --- a/fs/squashfs/squashfs.h
> +++ b/fs/squashfs/squashfs.h
> @@ -67,6 +67,7 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
>  				u64, u64, unsigned int);
>  
>  /* file.c */
> +void squashfs_fill_page(struct page *, struct squashfs_cache_entry *, int, int);
>  void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
>  				int);
>  
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team