2022-11-14 08:30:52

by Alexander Potapenko

Subject: [PATCH] fs: ext4: initialize fsdata in pagecache_write()

When aops->write_begin() does not initialize fsdata, KMSAN reports
an error passing the latter to aops->write_end().

Fix this by unconditionally initializing fsdata.

Also speculatively fix similar issues in affs, f2fs, hfs, hfsplus,
as suggested by Eric Biggers.

Cc: Eric Biggers <[email protected]>
Fixes: c93d8f885809 ("ext4: add basic fs-verity support")
Reported-by: [email protected]
Signed-off-by: Alexander Potapenko <[email protected]>
---
fs/affs/file.c | 2 +-
fs/ext4/verity.c | 2 +-
fs/f2fs/verity.c | 2 +-
fs/hfs/extent.c | 2 +-
fs/hfsplus/extents.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/affs/file.c b/fs/affs/file.c
index cefa222f7881c..8daeed31e1af9 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -880,7 +880,7 @@ affs_truncate(struct inode *inode)
if (inode->i_size > AFFS_I(inode)->mmu_private) {
struct address_space *mapping = inode->i_mapping;
struct page *page;
- void *fsdata;
+ void *fsdata = NULL;
loff_t isize = inode->i_size;
int res;

diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index 3c640bd7ecaeb..30e3b65798b50 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -79,7 +79,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
size_t n = min_t(size_t, count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
- void *fsdata;
+ void *fsdata = NULL;
int res;

res = aops->write_begin(NULL, mapping, pos, n, &page, &fsdata);
diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index c352fff88a5e6..3f4f3295f1c66 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -81,7 +81,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
size_t n = min_t(size_t, count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
- void *fsdata;
+ void *fsdata = NULL;
int res;

res = aops->write_begin(NULL, mapping, pos, n, &page, &fsdata);
diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 3f7e9bef98743..6d1878b99b305 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -486,7 +486,7 @@ void hfs_file_truncate(struct inode *inode)
inode->i_size);
if (inode->i_size > HFS_I(inode)->phys_size) {
struct address_space *mapping = inode->i_mapping;
- void *fsdata;
+ void *fsdata = NULL;
struct page *page;

/* XXX: Can use generic_cont_expand? */
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index 721f779b4ec3e..7a542f3dbe502 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -554,7 +554,7 @@ void hfsplus_file_truncate(struct inode *inode)
if (inode->i_size > hip->phys_size) {
struct address_space *mapping = inode->i_mapping;
struct page *page;
- void *fsdata;
+ void *fsdata = NULL;
loff_t size = inode->i_size;

res = hfsplus_write_begin(NULL, mapping, size, 0,
--
2.38.1.431.g37b22c650d-goog

2022-11-14 18:22:33

by Eric Biggers

Subject: Re: [PATCH] fs: ext4: initialize fsdata in pagecache_write()

On Mon, Nov 14, 2022 at 09:29:35AM +0100, Alexander Potapenko wrote:
> [PATCH] fs: ext4: initialize fsdata in pagecache_write()
>
> When aops->write_begin() does not initialize fsdata, KMSAN reports
> an error passing the latter to aops->write_end().
>
> Fix this by unconditionally initializing fsdata.
>
> Also speculatively fix similar issues in affs, f2fs, hfs, hfsplus,
> as suggested by Eric Biggers.

You might have better luck with separate patches for each filesystem, as it
might be hard to get someone to apply this patch otherwise.

If you do go with a single patch, then the subject prefix should be "fs:", not
"fs: ext4:".

- Eric

2022-11-21 11:30:49

by Alexander Potapenko

Subject: Re: [PATCH] fs: ext4: initialize fsdata in pagecache_write()

On Mon, Nov 14, 2022 at 7:20 PM Eric Biggers <[email protected]> wrote:
>
> On Mon, Nov 14, 2022 at 09:29:35AM +0100, Alexander Potapenko wrote:
> > [PATCH] fs: ext4: initialize fsdata in pagecache_write()
> >
> > When aops->write_begin() does not initialize fsdata, KMSAN reports
> > an error passing the latter to aops->write_end().
> >
> > Fix this by unconditionally initializing fsdata.
> >
> > Also speculatively fix similar issues in affs, f2fs, hfs, hfsplus,
> > as suggested by Eric Biggers.
>
> You might have better luck with separate patches for each filesystem, as it
> might be hard to get someone to apply this patch otherwise.

Done.
Please disregard this patch.

> If you do go with a single patch, then the subject prefix should be "fs:", not
> "fs: ext4:".
>
> - Eric



--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg