EROFS can select compression algorithms on a per-file basis, and each
per-file compression algorithm needs to be marked in the on-disk
superblock for initialization.
However, syzkaller can generate inconsistent crafted images that use
an unsupported algorithm for specific inodes; thus, an unexpected
"BUG: kernel NULL pointer dereference" can be raised.
Fix this by checking against `sbi->available_compr_algs` for each
compressed inode. Incorrect !erofs_sb_has_compr_cfgs preset bitmap
is now fixed together since it was harmless previously.
Reported-by: <[email protected]>
Fixes: 14373711dd54 ("erofs: add on-disk compression configurations")
Signed-off-by: Gao Xiang <[email protected]>
---
fs/erofs/decompressor.c | 2 +-
fs/erofs/zmap.c | 15 ++++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 021be5feb1bc..af98e88908ee 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -398,7 +398,7 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
int size, ret = 0;
if (!erofs_sb_has_compr_cfgs(sbi)) {
- sbi->available_compr_algs = Z_EROFS_COMPRESSION_LZ4;
+ sbi->available_compr_algs = 1 << Z_EROFS_COMPRESSION_LZ4;
return z_erofs_load_lz4_config(sb, dsb, NULL, 0);
}
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 7b55111fd533..d513f2cd7521 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -578,7 +578,8 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
struct super_block *const sb = inode->i_sb;
- int err, headnr;
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ int err, nr;
erofs_off_t pos;
struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
void *kaddr;
@@ -622,12 +623,12 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
vi->z_algorithmtype[0] = h->h_algorithmtype & 15;
vi->z_algorithmtype[1] = h->h_algorithmtype >> 4;
- headnr = 0;
- if (vi->z_algorithmtype[0] >= Z_EROFS_COMPRESSION_MAX ||
- vi->z_algorithmtype[++headnr] >= Z_EROFS_COMPRESSION_MAX) {
- erofs_err(sb, "unknown HEAD%u format %u for nid %llu, please upgrade kernel",
- headnr + 1, vi->z_algorithmtype[headnr], vi->nid);
- err = -EOPNOTSUPP;
+ nr = 0;
+ if (!(sbi->available_compr_algs & (1 << vi->z_algorithmtype[0])) ||
+ !(sbi->available_compr_algs & (1 << vi->z_algorithmtype[++nr]))) {
+ erofs_err(sb, "inconsistent HEAD%u algorithm format %u for nid %llu",
+ nr + 1, vi->z_algorithmtype[nr], vi->nid);
+ err = -EFSCORRUPTED;
goto out_put_metabuf;
}
--
2.39.3
EROFS can select compression algorithms on a per-file basis, and each
per-file compression algorithm needs to be marked in the on-disk
superblock for initialization.
However, syzkaller can generate inconsistent crafted images that use
an unsupported algorithm for specific inodes; thus, an unexpected
"BUG: kernel NULL pointer dereference" can be raised.
Fix this by checking against `sbi->available_compr_algs` for each
compressed inode. Incorrect !erofs_sb_has_compr_cfgs preset bitmap
is now fixed together since it was harmless previously.
Reported-by: <[email protected]>
Fixes: 14373711dd54 ("erofs: add on-disk compression configurations")
Signed-off-by: Gao Xiang <[email protected]>
---
change since v1:
- fix left-shift overflow (undefined behavior).
fs/erofs/decompressor.c | 2 +-
fs/erofs/zmap.c | 9 ++++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 021be5feb1bc..af98e88908ee 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -398,7 +398,7 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
int size, ret = 0;
if (!erofs_sb_has_compr_cfgs(sbi)) {
- sbi->available_compr_algs = Z_EROFS_COMPRESSION_LZ4;
+ sbi->available_compr_algs = 1 << Z_EROFS_COMPRESSION_LZ4;
return z_erofs_load_lz4_config(sb, dsb, NULL, 0);
}
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 7b55111fd533..6f10bc8bbb1c 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -578,6 +578,7 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
struct super_block *const sb = inode->i_sb;
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
int err, headnr;
erofs_off_t pos;
struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
@@ -624,10 +625,12 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
headnr = 0;
if (vi->z_algorithmtype[0] >= Z_EROFS_COMPRESSION_MAX ||
- vi->z_algorithmtype[++headnr] >= Z_EROFS_COMPRESSION_MAX) {
- erofs_err(sb, "unknown HEAD%u format %u for nid %llu, please upgrade kernel",
+ !(sbi->available_compr_algs & (1 << vi->z_algorithmtype[0])) ||
+ vi->z_algorithmtype[++headnr] >= Z_EROFS_COMPRESSION_MAX ||
+ !(sbi->available_compr_algs & (1 << vi->z_algorithmtype[headnr]))) {
+ erofs_err(sb, "inconsistent HEAD%u algorithm format %u for nid %llu",
headnr + 1, vi->z_algorithmtype[headnr], vi->nid);
- err = -EOPNOTSUPP;
+ err = -EFSCORRUPTED;
goto out_put_metabuf;
}
--
2.39.3
On Wed, 27 Dec 2023 13:06:33 +0800
Gao Xiang <[email protected]> wrote:
> EROFS can select compression algorithms on a per-file basis, and each
> per-file compression algorithm needs to be marked in the on-disk
> superblock for initialization.
>
> However, syzkaller can generate inconsistent crafted images that use
> an unsupported algorithm for specific inodes; thus, an unexpected
> "BUG: kernel NULL pointer dereference" can be raised.
>
> Fix this by checking against `sbi->available_compr_algs` for each
> compressed inode. Incorrect !erofs_sb_has_compr_cfgs preset bitmap
> is now fixed together since it was harmless previously.
>
> Reported-by: <[email protected]>
> Fixes: 14373711dd54 ("erofs: add on-disk compression configurations")
> Signed-off-by: Gao Xiang <[email protected]>
Reviewed-by: Yue Hu <[email protected]>
EROFS can select compression algorithms on a per-file basis, and each
per-file compression algorithm needs to be marked in the on-disk
superblock for initialization.
However, syzkaller can generate inconsistent crafted images that use
an unsupported algorithmtype for specific inodes, e.g. use MicroLZMA
algorithmtype even it's not set in `sbi->available_compr_algs`. This
can lead to an unexpected "BUG: kernel NULL pointer dereference" if
the corresponding decompressor isn't built-in.
Fix this by checking against `sbi->available_compr_algs` for each
m_algorithmformat request. Incorrect !erofs_sb_has_compr_cfgs preset
bitmap is now fixed together since it was harmless previously.
Reported-by: <[email protected]>
Fixes: 8f89926290c4 ("erofs: get compression algorithms directly on mapping")
Fixes: 622ceaddb764 ("erofs: lzma compression support")
Reviewed-by: Yue Hu <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---
changes since v2:
- Should check in z_erofs_do_map_blocks() runtimely since another
algorithmtype[0/1] could leave as unused (0) but it can be
problematic if LZ4 is not set in `sbi->available_compr_algs`.
fs/erofs/decompressor.c | 2 +-
fs/erofs/zmap.c | 23 +++++++++++++----------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 1d65b9f60a39..072ef6a66823 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -408,7 +408,7 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
int size, ret = 0;
if (!erofs_sb_has_compr_cfgs(sbi)) {
- sbi->available_compr_algs = Z_EROFS_COMPRESSION_LZ4;
+ sbi->available_compr_algs = 1 << Z_EROFS_COMPRESSION_LZ4;
return z_erofs_load_lz4_config(sb, dsb, NULL, 0);
}
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 9753875e41cb..7e1116804008 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -454,7 +454,7 @@ static int z_erofs_do_map_blocks(struct inode *inode,
.map = map,
};
int err = 0;
- unsigned int lclusterbits, endoff;
+ unsigned int lclusterbits, endoff, afmt;
unsigned long initial_lcn;
unsigned long long ofs, end;
@@ -543,17 +543,20 @@ static int z_erofs_do_map_blocks(struct inode *inode,
err = -EFSCORRUPTED;
goto unmap_out;
}
- if (vi->z_advise & Z_EROFS_ADVISE_INTERLACED_PCLUSTER)
- map->m_algorithmformat =
- Z_EROFS_COMPRESSION_INTERLACED;
- else
- map->m_algorithmformat =
- Z_EROFS_COMPRESSION_SHIFTED;
- } else if (m.headtype == Z_EROFS_LCLUSTER_TYPE_HEAD2) {
- map->m_algorithmformat = vi->z_algorithmtype[1];
+ afmt = vi->z_advise & Z_EROFS_ADVISE_INTERLACED_PCLUSTER ?
+ Z_EROFS_COMPRESSION_INTERLACED :
+ Z_EROFS_COMPRESSION_SHIFTED;
} else {
- map->m_algorithmformat = vi->z_algorithmtype[0];
+ afmt = m.headtype == Z_EROFS_LCLUSTER_TYPE_HEAD2 ?
+ vi->z_algorithmtype[1] : vi->z_algorithmtype[0];
+ if (!(EROFS_I_SB(inode)->available_compr_algs & (1 << afmt))) {
+ erofs_err(inode->i_sb, "inconsistent algorithmtype %u for nid %llu",
+ afmt, vi->nid);
+ err = -EFSCORRUPTED;
+ goto unmap_out;
+ }
}
+ map->m_algorithmformat = afmt;
if ((flags & EROFS_GET_BLOCKS_FIEMAP) ||
((flags & EROFS_GET_BLOCKS_READMORE) &&
--
2.39.3
On Sat, 13 Jan 2024 23:06:02 +0800
Gao Xiang <[email protected]> wrote:
> EROFS can select compression algorithms on a per-file basis, and each
> per-file compression algorithm needs to be marked in the on-disk
> superblock for initialization.
>
> However, syzkaller can generate inconsistent crafted images that use
> an unsupported algorithmtype for specific inodes, e.g. use MicroLZMA
> algorithmtype even it's not set in `sbi->available_compr_algs`. This
> can lead to an unexpected "BUG: kernel NULL pointer dereference" if
> the corresponding decompressor isn't built-in.
>
> Fix this by checking against `sbi->available_compr_algs` for each
> m_algorithmformat request. Incorrect !erofs_sb_has_compr_cfgs preset
> bitmap is now fixed together since it was harmless previously.
>
> Reported-by: <[email protected]>
> Fixes: 8f89926290c4 ("erofs: get compression algorithms directly on mapping")
> Fixes: 622ceaddb764 ("erofs: lzma compression support")
> Reviewed-by: Yue Hu <[email protected]>
> Signed-off-by: Gao Xiang <[email protected]>
LGTM.
Reviewed-by: Yue Hu <[email protected]>