Commit 9ef8eb61 authored by Phillip Lougher's avatar Phillip Lougher Committed by Andrew Morton

squashfs: fix read regression introduced in readahead code

Patch series "squashfs: fix some regressions introduced in the readahead
code".

This patchset fixes 3 regressions introduced by the recent readahead code
changes.  The first regression is causing "snaps" to randomly fail after a
couple of hours or days, which how the regression came to light.


This patch (of 3):

If a file isn't a whole multiple of the page size, the last page will have
trailing bytes unfilled.

There was a mistake in the readahead code which did this.  In particular
it incorrectly assumed that the last page in the readahead page array
(page[nr_pages - 1]) will always contain the last page in the block, which
if we're at file end, will be the page that needs to be zero filled.

But the readahead code may not return the last page in the block, which
means it is unmapped and will be skipped by the decompressors (a temporary
buffer used).

In this case the zero filling code will zero out the wrong page, leading
to data corruption.

Fix this by by extending the "page actor" to return the last page if
present, or NULL if a temporary buffer was used.

Link: https://lkml.kernel.org/r/20221020223616.7571-1-phillip@squashfs.org.uk
Link: https://lkml.kernel.org/r/20221020223616.7571-2-phillip@squashfs.org.uk
Fixes: 8fc78b6f ("squashfs: implement readahead")
Link: https://lore.kernel.org/lkml/b0c258c3-6dcf-aade-efc4-d62a8b3a1ce2@alu.unizg.hr/Signed-off-by: default avatarPhillip Lougher <phillip@squashfs.org.uk>
Reported-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Tested-by: default avatarMirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Tested-by: default avatarSlade Watkins <srw@sladewatkins.net>
Tested-by: default avatarBagas Sanjaya <bagasdotme@gmail.com>
Reported-by: default avatarMarc Miltenberger <marcmiltenberger@gmail.com>
Cc: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Thorsten Leemhuis <regressions@leemhuis.info>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 97061d44
...@@ -557,6 +557,7 @@ static void squashfs_readahead(struct readahead_control *ractl) ...@@ -557,6 +557,7 @@ static void squashfs_readahead(struct readahead_control *ractl)
int res, bsize; int res, bsize;
u64 block = 0; u64 block = 0;
unsigned int expected; unsigned int expected;
struct page *last_page;
nr_pages = __readahead_batch(ractl, pages, max_pages); nr_pages = __readahead_batch(ractl, pages, max_pages);
if (!nr_pages) if (!nr_pages)
...@@ -593,15 +594,15 @@ static void squashfs_readahead(struct readahead_control *ractl) ...@@ -593,15 +594,15 @@ static void squashfs_readahead(struct readahead_control *ractl)
res = squashfs_read_data(inode->i_sb, block, bsize, NULL, actor); res = squashfs_read_data(inode->i_sb, block, bsize, NULL, actor);
squashfs_page_actor_free(actor); last_page = squashfs_page_actor_free(actor);
if (res == expected) { if (res == expected) {
int bytes; int bytes;
/* Last page (if present) may have trailing bytes not filled */ /* Last page (if present) may have trailing bytes not filled */
bytes = res % PAGE_SIZE; bytes = res % PAGE_SIZE;
if (pages[nr_pages - 1]->index == file_end && bytes) if (index == file_end && bytes && last_page)
memzero_page(pages[nr_pages - 1], bytes, memzero_page(last_page, bytes,
PAGE_SIZE - bytes); PAGE_SIZE - bytes);
for (i = 0; i < nr_pages; i++) { for (i = 0; i < nr_pages; i++) {
......
...@@ -71,11 +71,13 @@ static void *handle_next_page(struct squashfs_page_actor *actor) ...@@ -71,11 +71,13 @@ static void *handle_next_page(struct squashfs_page_actor *actor)
(actor->next_index != actor->page[actor->next_page]->index)) { (actor->next_index != actor->page[actor->next_page]->index)) {
actor->next_index++; actor->next_index++;
actor->returned_pages++; actor->returned_pages++;
actor->last_page = NULL;
return actor->alloc_buffer ? actor->tmp_buffer : ERR_PTR(-ENOMEM); return actor->alloc_buffer ? actor->tmp_buffer : ERR_PTR(-ENOMEM);
} }
actor->next_index++; actor->next_index++;
actor->returned_pages++; actor->returned_pages++;
actor->last_page = actor->page[actor->next_page];
return actor->pageaddr = kmap_local_page(actor->page[actor->next_page++]); return actor->pageaddr = kmap_local_page(actor->page[actor->next_page++]);
} }
...@@ -125,6 +127,7 @@ struct squashfs_page_actor *squashfs_page_actor_init_special(struct squashfs_sb_ ...@@ -125,6 +127,7 @@ struct squashfs_page_actor *squashfs_page_actor_init_special(struct squashfs_sb_
actor->returned_pages = 0; actor->returned_pages = 0;
actor->next_index = page[0]->index & ~((1 << (msblk->block_log - PAGE_SHIFT)) - 1); actor->next_index = page[0]->index & ~((1 << (msblk->block_log - PAGE_SHIFT)) - 1);
actor->pageaddr = NULL; actor->pageaddr = NULL;
actor->last_page = NULL;
actor->alloc_buffer = msblk->decompressor->alloc_buffer; actor->alloc_buffer = msblk->decompressor->alloc_buffer;
actor->squashfs_first_page = direct_first_page; actor->squashfs_first_page = direct_first_page;
actor->squashfs_next_page = direct_next_page; actor->squashfs_next_page = direct_next_page;
......
...@@ -16,6 +16,7 @@ struct squashfs_page_actor { ...@@ -16,6 +16,7 @@ struct squashfs_page_actor {
void *(*squashfs_first_page)(struct squashfs_page_actor *); void *(*squashfs_first_page)(struct squashfs_page_actor *);
void *(*squashfs_next_page)(struct squashfs_page_actor *); void *(*squashfs_next_page)(struct squashfs_page_actor *);
void (*squashfs_finish_page)(struct squashfs_page_actor *); void (*squashfs_finish_page)(struct squashfs_page_actor *);
struct page *last_page;
int pages; int pages;
int length; int length;
int next_page; int next_page;
...@@ -29,10 +30,13 @@ extern struct squashfs_page_actor *squashfs_page_actor_init(void **buffer, ...@@ -29,10 +30,13 @@ extern struct squashfs_page_actor *squashfs_page_actor_init(void **buffer,
extern struct squashfs_page_actor *squashfs_page_actor_init_special( extern struct squashfs_page_actor *squashfs_page_actor_init_special(
struct squashfs_sb_info *msblk, struct squashfs_sb_info *msblk,
struct page **page, int pages, int length); struct page **page, int pages, int length);
static inline void squashfs_page_actor_free(struct squashfs_page_actor *actor) static inline struct page *squashfs_page_actor_free(struct squashfs_page_actor *actor)
{ {
struct page *last_page = actor->last_page;
kfree(actor->tmp_buffer); kfree(actor->tmp_buffer);
kfree(actor); kfree(actor);
return last_page;
} }
static inline void *squashfs_first_page(struct squashfs_page_actor *actor) static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment