The patch titled
Subject: squashfs: fix buffer release race condition in readahead code
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
squashfs-fix-buffer-release-race-condition-in-readahead-code.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patche…
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Phillip Lougher <phillip(a)squashfs.org.uk>
Subject: squashfs: fix buffer release race condition in readahead code
Date: Thu, 20 Oct 2022 23:36:16 +0100
Fix a buffer release race condition, where the error value was used after
release.
Link: https://lkml.kernel.org/r/20221020223616.7571-4-phillip@squashfs.org.uk
Fixes: b09a7a036d20 ("squashfs: support reading fragments in readahead call")
Signed-off-by: Phillip Lougher <phillip(a)squashfs.org.uk>
Cc: <stable(a)vger.kernel.org>
Cc: Bagas Sanjaya <bagasdotme(a)gmail.com>
Cc: Dimitri John Ledkov <dimitri.ledkov(a)canonical.com>
Cc: Hsin-Yi Wang <hsinyi(a)chromium.org>
Cc: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Cc: Slade Watkins <srw(a)sladewatkins.net>
Cc: Thorsten Leemhuis <regressions(a)leemhuis.info>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/squashfs/file.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/fs/squashfs/file.c~squashfs-fix-buffer-release-race-condition-in-readahead-code
+++ a/fs/squashfs/file.c
@@ -506,8 +506,9 @@ static int squashfs_readahead_fragment(s
squashfs_i(inode)->fragment_size);
struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
unsigned int n, mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
+ int error = buffer->error;
- if (buffer->error)
+ if (error)
goto out;
expected += squashfs_i(inode)->fragment_offset;
@@ -529,7 +530,7 @@ static int squashfs_readahead_fragment(s
out:
squashfs_cache_put(buffer);
- return buffer->error;
+ return error;
}
static void squashfs_readahead(struct readahead_control *ractl)
_
Patches currently in -mm which might be from phillip(a)squashfs.org.uk are
squashfs-fix-read-regression-introduced-in-readahead-code.patch
squashfs-fix-extending-readahead-beyond-end-of-file.patch
squashfs-fix-buffer-release-race-condition-in-readahead-code.patch
The patch titled
Subject: squashfs: fix extending readahead beyond end of file
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
squashfs-fix-extending-readahead-beyond-end-of-file.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patche…
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Phillip Lougher <phillip(a)squashfs.org.uk>
Subject: squashfs: fix extending readahead beyond end of file
Date: Thu, 20 Oct 2022 23:36:15 +0100
The readahead code will try to extend readahead to the entire size of the
Squashfs data block.
But, it didn't take into account that the last block at the end of the
file may not be a whole block. In this case, the code would extend
readahead to beyond the end of the file, leaving trailing pages.
Fix this by only requesting the expected number of pages.
Link: https://lkml.kernel.org/r/20221020223616.7571-3-phillip@squashfs.org.uk
Fixes: 8fc78b6fe24c ("squashfs: implement readahead")
Signed-off-by: Phillip Lougher <phillip(a)squashfs.org.uk>
Cc: <stable(a)vger.kernel.org>
Cc: Bagas Sanjaya <bagasdotme(a)gmail.com>
Cc: Dimitri John Ledkov <dimitri.ledkov(a)canonical.com>
Cc: Hsin-Yi Wang <hsinyi(a)chromium.org>
Cc: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Cc: Slade Watkins <srw(a)sladewatkins.net>
Cc: Thorsten Leemhuis <regressions(a)leemhuis.info>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/squashfs/file.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--- a/fs/squashfs/file.c~squashfs-fix-extending-readahead-beyond-end-of-file
+++ a/fs/squashfs/file.c
@@ -559,6 +559,12 @@ static void squashfs_readahead(struct re
unsigned int expected;
struct page *last_page;
+ expected = start >> msblk->block_log == file_end ?
+ (i_size_read(inode) & (msblk->block_size - 1)) :
+ msblk->block_size;
+
+ max_pages = (expected + PAGE_SIZE - 1) >> PAGE_SHIFT;
+
nr_pages = __readahead_batch(ractl, pages, max_pages);
if (!nr_pages)
break;
@@ -567,13 +573,10 @@ static void squashfs_readahead(struct re
goto skip_pages;
index = pages[0]->index >> shift;
+
if ((pages[nr_pages - 1]->index >> shift) != index)
goto skip_pages;
- expected = index == file_end ?
- (i_size_read(inode) & (msblk->block_size - 1)) :
- msblk->block_size;
-
if (index == file_end && squashfs_i(inode)->fragment_block !=
SQUASHFS_INVALID_BLK) {
res = squashfs_readahead_fragment(pages, nr_pages,
_
Patches currently in -mm which might be from phillip(a)squashfs.org.uk are
squashfs-fix-read-regression-introduced-in-readahead-code.patch
squashfs-fix-extending-readahead-beyond-end-of-file.patch
squashfs-fix-buffer-release-race-condition-in-readahead-code.patch
The patch titled
Subject: squashfs: fix read regression introduced in readahead code
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
squashfs-fix-read-regression-introduced-in-readahead-code.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patche…
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Phillip Lougher <phillip(a)squashfs.org.uk>
Subject: squashfs: fix read regression introduced in readahead code
Date: Thu, 20 Oct 2022 23:36:14 +0100
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: 8fc78b6fe24c ("squashfs: implement readahead")
Link: https://lore.kernel.org/lkml/b0c258c3-6dcf-aade-efc4-d62a8b3a1ce2@alu.unizg…
Signed-off-by: Phillip Lougher <phillip(a)squashfs.org.uk>
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Tested-by: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Tested-by: Slade Watkins <srw(a)sladewatkins.net>
Tested-by: Bagas Sanjaya <bagasdotme(a)gmail.com>
Cc: <stable(a)vger.kernel.org>
Cc: Dimitri John Ledkov <dimitri.ledkov(a)canonical.com>
Cc: Hsin-Yi Wang <hsinyi(a)chromium.org>
Cc: Thorsten Leemhuis <regressions(a)leemhuis.info>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/squashfs/file.c | 7 ++++---
fs/squashfs/page_actor.c | 3 +++
fs/squashfs/page_actor.h | 6 +++++-
3 files changed, 12 insertions(+), 4 deletions(-)
--- a/fs/squashfs/file.c~squashfs-fix-read-regression-introduced-in-readahead-code
+++ a/fs/squashfs/file.c
@@ -557,6 +557,7 @@ static void squashfs_readahead(struct re
int res, bsize;
u64 block = 0;
unsigned int expected;
+ struct page *last_page;
nr_pages = __readahead_batch(ractl, pages, max_pages);
if (!nr_pages)
@@ -593,15 +594,15 @@ static void squashfs_readahead(struct re
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) {
int bytes;
/* Last page (if present) may have trailing bytes not filled */
bytes = res % PAGE_SIZE;
- if (pages[nr_pages - 1]->index == file_end && bytes)
- memzero_page(pages[nr_pages - 1], bytes,
+ if (index == file_end && bytes && last_page)
+ memzero_page(last_page, bytes,
PAGE_SIZE - bytes);
for (i = 0; i < nr_pages; i++) {
--- a/fs/squashfs/page_actor.c~squashfs-fix-read-regression-introduced-in-readahead-code
+++ a/fs/squashfs/page_actor.c
@@ -71,11 +71,13 @@ static void *handle_next_page(struct squ
(actor->next_index != actor->page[actor->next_page]->index)) {
actor->next_index++;
actor->returned_pages++;
+ actor->last_page = NULL;
return actor->alloc_buffer ? actor->tmp_buffer : ERR_PTR(-ENOMEM);
}
actor->next_index++;
actor->returned_pages++;
+ actor->last_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_pag
actor->returned_pages = 0;
actor->next_index = page[0]->index & ~((1 << (msblk->block_log - PAGE_SHIFT)) - 1);
actor->pageaddr = NULL;
+ actor->last_page = NULL;
actor->alloc_buffer = msblk->decompressor->alloc_buffer;
actor->squashfs_first_page = direct_first_page;
actor->squashfs_next_page = direct_next_page;
--- a/fs/squashfs/page_actor.h~squashfs-fix-read-regression-introduced-in-readahead-code
+++ a/fs/squashfs/page_actor.h
@@ -16,6 +16,7 @@ struct squashfs_page_actor {
void *(*squashfs_first_page)(struct squashfs_page_actor *);
void *(*squashfs_next_page)(struct squashfs_page_actor *);
void (*squashfs_finish_page)(struct squashfs_page_actor *);
+ struct page *last_page;
int pages;
int length;
int next_page;
@@ -29,10 +30,13 @@ extern struct squashfs_page_actor *squas
extern struct squashfs_page_actor *squashfs_page_actor_init_special(
struct squashfs_sb_info *msblk,
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);
+ return last_page;
}
static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
{
_
Patches currently in -mm which might be from phillip(a)squashfs.org.uk are
squashfs-fix-read-regression-introduced-in-readahead-code.patch
squashfs-fix-extending-readahead-beyond-end-of-file.patch
squashfs-fix-buffer-release-race-condition-in-readahead-code.patch
The readahead code will try to extend readahead to the entire
size of the Squashfs data block.
But, it didn't take into account that the last block at the end of
the file may not be a whole block. In this case, the code would
extend readahead to beyond the end of the file, leaving trailing
pages.
Fix this by only requesting the expected number of pages.
Fixes: 8fc78b6fe24c ("squashfs: implement readahead")
Signed-off-by: Phillip Lougher <phillip(a)squashfs.org.uk>
Cc: <stable(a)vger.kernel.org>
---
fs/squashfs/file.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index e526eb7a1658..f0afd4d6fd30 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -559,6 +559,12 @@ static void squashfs_readahead(struct readahead_control *ractl)
unsigned int expected;
struct page *last_page;
+ expected = start >> msblk->block_log == file_end ?
+ (i_size_read(inode) & (msblk->block_size - 1)) :
+ msblk->block_size;
+
+ max_pages = (expected + PAGE_SIZE - 1) >> PAGE_SHIFT;
+
nr_pages = __readahead_batch(ractl, pages, max_pages);
if (!nr_pages)
break;
@@ -567,13 +573,10 @@ static void squashfs_readahead(struct readahead_control *ractl)
goto skip_pages;
index = pages[0]->index >> shift;
+
if ((pages[nr_pages - 1]->index >> shift) != index)
goto skip_pages;
- expected = index == file_end ?
- (i_size_read(inode) & (msblk->block_size - 1)) :
- msblk->block_size;
-
if (index == file_end && squashfs_i(inode)->fragment_block !=
SQUASHFS_INVALID_BLK) {
res = squashfs_readahead_fragment(pages, nr_pages,
--
2.35.1
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.
Fixes: 8fc78b6fe24c ("squashfs: implement readahead")
Link: https://lore.kernel.org/lkml/b0c258c3-6dcf-aade-efc4-d62a8b3a1ce2@alu.unizg…
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Tested-by: Mirsad Goran Todorovac <mirsad.todorovac(a)alu.unizg.hr>
Tested-by: Slade Watkins <srw(a)sladewatkins.net>
Tested-by: Bagas Sanjaya <bagasdotme(a)gmail.com>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Phillip Lougher <phillip(a)squashfs.org.uk>
---
fs/squashfs/file.c | 7 ++++---
fs/squashfs/page_actor.c | 3 +++
fs/squashfs/page_actor.h | 6 +++++-
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index e56510964b22..e526eb7a1658 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -557,6 +557,7 @@ static void squashfs_readahead(struct readahead_control *ractl)
int res, bsize;
u64 block = 0;
unsigned int expected;
+ struct page *last_page;
nr_pages = __readahead_batch(ractl, pages, max_pages);
if (!nr_pages)
@@ -593,15 +594,15 @@ static void squashfs_readahead(struct readahead_control *ractl)
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) {
int bytes;
/* Last page (if present) may have trailing bytes not filled */
bytes = res % PAGE_SIZE;
- if (pages[nr_pages - 1]->index == file_end && bytes)
- memzero_page(pages[nr_pages - 1], bytes,
+ if (index == file_end && bytes && last_page)
+ memzero_page(last_page, bytes,
PAGE_SIZE - bytes);
for (i = 0; i < nr_pages; i++) {
diff --git a/fs/squashfs/page_actor.c b/fs/squashfs/page_actor.c
index 54b93bf4a25c..81af6c4ca115 100644
--- a/fs/squashfs/page_actor.c
+++ b/fs/squashfs/page_actor.c
@@ -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->returned_pages++;
+ actor->last_page = NULL;
return actor->alloc_buffer ? actor->tmp_buffer : ERR_PTR(-ENOMEM);
}
actor->next_index++;
actor->returned_pages++;
+ actor->last_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_
actor->returned_pages = 0;
actor->next_index = page[0]->index & ~((1 << (msblk->block_log - PAGE_SHIFT)) - 1);
actor->pageaddr = NULL;
+ actor->last_page = NULL;
actor->alloc_buffer = msblk->decompressor->alloc_buffer;
actor->squashfs_first_page = direct_first_page;
actor->squashfs_next_page = direct_next_page;
diff --git a/fs/squashfs/page_actor.h b/fs/squashfs/page_actor.h
index 95ffbb543d91..97d4983559b1 100644
--- a/fs/squashfs/page_actor.h
+++ b/fs/squashfs/page_actor.h
@@ -16,6 +16,7 @@ struct squashfs_page_actor {
void *(*squashfs_first_page)(struct squashfs_page_actor *);
void *(*squashfs_next_page)(struct squashfs_page_actor *);
void (*squashfs_finish_page)(struct squashfs_page_actor *);
+ struct page *last_page;
int pages;
int length;
int next_page;
@@ -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(
struct squashfs_sb_info *msblk,
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);
+ return last_page;
}
static inline void *squashfs_first_page(struct squashfs_page_actor *actor)
{
--
2.35.1
On Wed, Oct 05, 2022, Ferry Toth wrote:
> Hi,
>
> Thanks!
>
> Does the failure only happen the first time host is initialized? Or can
> it recover after switching to device then back to host mode?
>
> I can switch back and forth and device mode works each time, host mode remains
> dead.
Ok.
>
> Probably the failure happens if some step(s) in dwc3_core_init() hasn't
> completed.
>
> tusb1210 is a phy driver right? The issue is probably because we didn't
> initialize the phy yet. So, I suspect placing dwc3_get_extcon() after
> initializing the phy will probably solve the dependency problem.
>
> You can try something for yourself or I can provide something to test
> later if you don't mind (maybe next week if it's ok).
>
> Yes, the code move I mentioned above "moves dwc3_get_extcon() until after
> dwc3_core_init() but just before dwc3_core_init_mode(). AFAIU initially
> dwc3_get_extcon() was called from within dwc3_core_init_mode() but only for
> case USB_DR_MODE_OTG. So with this change order of events is more or less
> unchanged" solves the issue.
>
I saw the experiment you did from the link you provided. We want to also
confirm exactly which step in dwc3_core_init() was needed.
Thanks,
Thinh