Bits, which are related to Bitmap Descriptor logical blocks,
are not reset when buffer headers are allocated for them. As the
result, these logical blocks can be treated as free and
be used for other blocks.This can cause usage of one buffer header
for several types of data. UDF issues WARNING in this situation:
WARNING: CPU: 0 PID: 2703 at fs/udf/inode.c:2014
__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
RIP: 0010:__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
Call Trace:
udf_setup_indirect_aext+0x573/0x880 fs/udf/inode.c:1980
udf_add_aext+0x208/0x2e0 fs/udf/inode.c:2067
udf_insert_aext fs/udf/inode.c:2233 [inline]
udf_update_extents fs/udf/inode.c:1181 [inline]
inode_getblk+0x1981/0x3b70 fs/udf/inode.c:885
Found by Linux Verification Center (linuxtesting.org) with syzkaller.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Vladislav Efanov <[email protected]>
---
fs/udf/balloc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 8e597db4d971..52dab5b63715 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -37,6 +37,7 @@ static int read_block_bitmap(struct super_block *sb,
{
struct buffer_head *bh = NULL;
int retval = 0;
+ int i;
struct kernel_lb_addr loc;
loc.logicalBlockNum = bitmap->s_extPosition;
@@ -47,6 +48,12 @@ static int read_block_bitmap(struct super_block *sb,
retval = -EIO;
bitmap->s_block_bitmap[bitmap_nr] = bh;
+ /* Reserve bits for Space Bitmap buffer headers. */
+ if (bh && !bitmap_nr)
+ for (i = 0; i < bitmap->s_nr_groups; i++)
+ udf_clear_bit(bitmap->s_extPosition + i +
+ (sizeof(struct spaceBitmapDesc) << 3),
+ bh->b_data);
return retval;
}
--
2.34.1
On Fri 20-01-23 12:08:58, Vladislav Efanov wrote:
> Bits, which are related to Bitmap Descriptor logical blocks,
> are not reset when buffer headers are allocated for them. As the
> result, these logical blocks can be treated as free and
> be used for other blocks.This can cause usage of one buffer header
> for several types of data. UDF issues WARNING in this situation:
>
> WARNING: CPU: 0 PID: 2703 at fs/udf/inode.c:2014
> __udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
>
> RIP: 0010:__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
> Call Trace:
> udf_setup_indirect_aext+0x573/0x880 fs/udf/inode.c:1980
> udf_add_aext+0x208/0x2e0 fs/udf/inode.c:2067
> udf_insert_aext fs/udf/inode.c:2233 [inline]
> udf_update_extents fs/udf/inode.c:1181 [inline]
> inode_getblk+0x1981/0x3b70 fs/udf/inode.c:885
>
> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Vladislav Efanov <[email protected]>
Thanks for the analysis and the fix. Two notes below:
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 8e597db4d971..52dab5b63715 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -37,6 +37,7 @@ static int read_block_bitmap(struct super_block *sb,
> {
> struct buffer_head *bh = NULL;
> int retval = 0;
> + int i;
> struct kernel_lb_addr loc;
>
> loc.logicalBlockNum = bitmap->s_extPosition;
> @@ -47,6 +48,12 @@ static int read_block_bitmap(struct super_block *sb,
> retval = -EIO;
>
> bitmap->s_block_bitmap[bitmap_nr] = bh;
> + /* Reserve bits for Space Bitmap buffer headers. */
> + if (bh && !bitmap_nr)
> + for (i = 0; i < bitmap->s_nr_groups; i++)
> + udf_clear_bit(bitmap->s_extPosition + i +
> + (sizeof(struct spaceBitmapDesc) << 3),
> + bh->b_data);
> return retval;
> }
Rather than just siletly making blocks allocated, we should test whether
the block is allocated and if not, return an error (-EFSCORRUPTED) and
refuse to use the filesystem.
Also with 512-byte blocksize one block describes 2MB of the filesystem so
for a filesystem larger than 8GB bitmap->s_nr_groups can be larger than a
number of bits in a block. So we need to be a bit more careful when
verifying the bitmap.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
Greeting,
FYI, we noticed BUG:KASAN:use-after-free_in_load_block_bitmap due to commit (built with gcc-11):
commit: e8161db9511ffda382e586a9a1f87ccb2b479ca8 ("[PATCH] udf: Reserve bits for Bitmap Descriptor buffers")
url: https://github.com/intel-lab-lkp/linux/commits/Vladislav-Efanov/udf-Reserve-bits-for-Bitmap-Descriptor-buffers/20230120-172019
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git d368967cb1039b5c4cccb62b5a4b9468c50cd143
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH] udf: Reserve bits for Bitmap Descriptor buffers
in testcase: xfstests
version: xfstests-x86_64-fb6575e-1_20230116
with following parameters:
disk: 4HDD
fs: udf
test: generic-group-24
test-description: xfstests is a regression test suite for xfs and other files ystems.
test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
[ 63.221412][ T1358] run fstests generic/490 at 2023-01-24 10:29:18
[ 63.788011][ T3618] ==================================================================
[ 63.795875][ T3618] BUG: KASAN: use-after-free in load_block_bitmap+0x291/0x370 [udf]
[ 63.803652][ T3618] Write of size 8 at addr ffff8881ffdbf000 by task seek_sanity_tes/3618
[ 63.811775][ T3618]
[ 63.813958][ T3618] CPU: 1 PID: 3618 Comm: seek_sanity_tes Tainted: G I 6.2.0-rc4-00078-ge8161db9511f #1
[ 63.824758][ T3618] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.1.1 10/07/2015
[ 63.832783][ T3618] Call Trace:
[ 63.835903][ T3618] <TASK>
[ 63.838675][ T3618] dump_stack_lvl+0x38/0x48
[ 63.843021][ T3618] print_address_description+0x87/0x2a1
[ 63.849415][ T3618] print_report+0x103/0x1e9
[ 63.853747][ T3618] ? kasan_addr_to_slab+0xd/0xa0
[ 63.858514][ T3618] ? load_block_bitmap+0x291/0x370 [udf]
[ 63.863963][ T3618] kasan_report+0xb2/0xe0
[ 63.868115][ T3618] ? load_block_bitmap+0x291/0x370 [udf]
[ 63.873565][ T3618] kasan_check_range+0x39/0x1b0
[ 63.878233][ T3618] load_block_bitmap+0x291/0x370 [udf]
[ 63.883516][ T3618] udf_bitmap_new_block+0x1e9/0xe00 [udf]
[ 63.889059][ T3618] ? udf_do_extend_file+0x353/0x590 [udf]
[ 63.894604][ T3618] ? udf_next_aext+0x3a0/0x3a0 [udf]
[ 63.899729][ T3618] udf_new_block+0x185/0x250 [udf]
[ 63.904699][ T3618] inode_getblk+0xc37/0x1710 [udf]
[ 63.909653][ T3618] ? kasan_save_stack+0x35/0x40
[ 63.914323][ T3618] ? kasan_save_stack+0x22/0x40
[ 63.918993][ T3618] ? udf_update_extents+0x4d0/0x4d0 [udf]
[ 63.924528][ T3618] ? entry_SYSCALL_64_after_hwframe+0x5e/0xc8
[ 63.930404][ T3618] ? xas_create+0xd4/0x7e0
[ 63.934644][ T3618] ? mem_cgroup_handle_over_high+0x480/0x480
[ 63.940434][ T3618] ? __mod_lruvec_page_state+0x1a9/0x420
[ 63.945879][ T3618] ? _raw_spin_lock+0x85/0xe0
[ 63.950375][ T3618] ? _raw_write_lock_irq+0xe0/0xe0
[ 63.955302][ T3618] ? down_write_killable+0x160/0x160
[ 63.960402][ T3618] udf_get_block+0x162/0x580 [udf]
[ 63.965336][ T3618] ? udf_block_map+0x250/0x250 [udf]
[ 63.970443][ T3618] __block_write_begin_int+0x290/0x880
[ 63.975728][ T3618] ? udf_block_map+0x250/0x250 [udf]
[ 63.980847][ T3618] ? invalidate_bh_lrus_cpu+0x80/0x80
[ 63.986032][ T3618] ? truncate_inode_partial_folio+0x390/0x390
[ 63.991908][ T3618] ? udf_block_map+0x250/0x250 [udf]
[ 63.997011][ T3618] block_write_begin+0x77/0x2c0
[ 64.001698][ T3618] udf_write_begin+0x30/0x90 [udf]
[ 64.006644][ T3618] generic_perform_write+0x21e/0x4b0
[ 64.011757][ T3618] ? filemap_fdatawrite_wbc+0x180/0x180
[ 64.017114][ T3618] ? current_time+0x77/0x220
[ 64.021526][ T3618] ? alloc_inode+0x1e0/0x1e0
[ 64.025936][ T3618] ? __cond_resched+0x20/0x90
[ 64.030432][ T3618] __generic_file_write_iter+0x218/0x420
[ 64.035875][ T3618] ? udf_setsize+0x4df/0x6b0 [udf]
[ 64.040814][ T3618] udf_file_write_iter+0x239/0x550 [udf]
[ 64.046263][ T3618] vfs_write+0x788/0xb60
[ 64.050331][ T3618] ? kernel_write+0x430/0x430
[ 64.054828][ T3618] ? up_write+0x53/0x90
[ 64.058812][ T3618] __x64_sys_pwrite64+0x19a/0x1f0
[ 64.063654][ T3618] ? vfs_write+0xb60/0xb60
[ 64.067891][ T3618] ? do_sys_ftruncate+0x3b6/0x490
[ 64.072735][ T3618] do_syscall_64+0x39/0x80
[ 64.076973][ T3618] entry_SYSCALL_64_after_hwframe+0x5e/0xc8
[ 64.082676][ T3618] RIP: 0033:0x7fa8533b3986
[ 64.086929][ T3618] Code: 48 c7 c0 ff ff ff ff eb ba 66 2e 0f 1f 84 00 00 00 00 00 90 49 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 11 b8 12 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 5a c3 90 48 83 ec 28 48 89 54 24 10 48 89 74
[ 64.106239][ T3618] RSP: 002b:00007fff3cc60608 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
[ 64.114439][ T3618] RAX: ffffffffffffffda RBX: 0000000000000200 RCX: 00007fa8533b3986
[ 64.122213][ T3618] RDX: 0000000000000001 RSI: 00005587b53599ec RDI: 0000000000000004
[ 64.129993][ T3618] RBP: 0000000000000004 R08: 0000000000000001 R09: 000000000000001f
[ 64.137775][ T3618] R10: 0000000000000200 R11: 0000000000000246 R12: 00005587b53599ec
[ 64.145556][ T3618] R13: 0000000000000001 R14: 0000000000000001 R15: 000000000000000c
[ 64.153328][ T3618] </TASK>
[ 64.156184][ T3618]
[ 64.158354][ T3618] The buggy address belongs to the physical page:
[ 64.164572][ T3618] page:00000000b46d5236 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x1ffdbf
[ 64.174582][ T3618] flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
[ 64.181755][ T3618] raw: 0017ffffc0000000 ffffea0008014b88 ffffea0004f41e08 0000000000000000
[ 64.190128][ T3618] raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
[ 64.198509][ T3618] page dumped because: kasan: bad access detected
[ 64.204736][ T3618]
[ 64.206916][ T3618] Memory state around the buggy address:
[ 64.212359][ T3618] ffff8881ffdbef00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 64.220215][ T3618] ffff8881ffdbef80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 64.228069][ T3618] >ffff8881ffdbf000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 64.235922][ T3618] ^
[ 64.239814][ T3618] ffff8881ffdbf080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 64.247668][ T3618] ffff8881ffdbf100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 64.255520][ T3618] ==================================================================
[ 64.263404][ T3618] Disabling lock debugging due to kernel taint
[ 64.381695][ T253] generic/490 _check_dmesg: something found in dmesg (see /lkp/benchmarks/xfstests/results//generic/490.dmesg)
If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-lkp/[email protected]
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Bits, which are related to Bitmap Descriptor logical blocks,
are not reset when buffer headers are allocated for them. As the
result, these logical blocks can be treated as free and
be used for other blocks.This can cause usage of one buffer header
for several types of data. UDF issues WARNING in this situation:
WARNING: CPU: 0 PID: 2703 at fs/udf/inode.c:2014
__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
RIP: 0010:__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
Call Trace:
udf_setup_indirect_aext+0x573/0x880 fs/udf/inode.c:1980
udf_add_aext+0x208/0x2e0 fs/udf/inode.c:2067
udf_insert_aext fs/udf/inode.c:2233 [inline]
udf_update_extents fs/udf/inode.c:1181 [inline]
inode_getblk+0x1981/0x3b70 fs/udf/inode.c:885
Found by Linux Verification Center (linuxtesting.org) with syzkaller.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Vladislav Efanov <[email protected]>
---
v2: Do not clear bits related to Bitmap Descriptor logical blocks,
but return -EFSCORRUPTED error instead.
fs/udf/balloc.c | 24 ++++++++++++++++++++++++
fs/udf/udfdecl.h | 1 +
2 files changed, 25 insertions(+)
diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 8e597db4d971..ce28d2567f91 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -37,6 +37,11 @@ static int read_block_bitmap(struct super_block *sb,
{
struct buffer_head *bh = NULL;
int retval = 0;
+ int i;
+ int max_bits = 0;
+ int max_bits_1st;
+ int max_bits_others;
+ int rest_bits;
struct kernel_lb_addr loc;
loc.logicalBlockNum = bitmap->s_extPosition;
@@ -47,6 +52,25 @@ static int read_block_bitmap(struct super_block *sb,
retval = -EIO;
bitmap->s_block_bitmap[bitmap_nr] = bh;
+ /* Check consistency of Space Bitmap buffer. */
+ if (bh) {
+ max_bits_others = sb->s_blocksize * 8;
+ max_bits_1st = max_bits_others - (sizeof(struct spaceBitmapDesc) << 3);
+ rest_bits = (bitmap->s_nr_groups > max_bits_1st) ?
+ bitmap->s_nr_groups - max_bits_1st : 0;
+ if (!bitmap_nr)
+ max_bits = min(max_bits_1st, bitmap->s_nr_groups);
+ else if (bitmap_nr < rest_bits / max_bits_others + 1)
+ max_bits = max_bits_others;
+ else if (bitmap_nr == rest_bits / max_bits_others + 1)
+ max_bits = rest_bits % max_bits_others;
+ for (i = 0; i < max_bits; i++) {
+ if (udf_test_bit(i + (bitmap_nr ? 0 :
+ (sizeof(struct spaceBitmapDesc) << 3)),
+ bh->b_data))
+ return -EFSCORRUPTED;
+ }
+ }
return retval;
}
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 7e258f15b8ef..130290ae3329 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -16,6 +16,7 @@
#include "udfend.h"
#include "udf_i.h"
+#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
#define UDF_DEFAULT_PREALLOC_BLOCKS 8
extern __printf(3, 4) void _udf_err(struct super_block *sb,
--
2.34.1
On Thu 02-02-23 17:04:56, Vladislav Efanov wrote:
> Bits, which are related to Bitmap Descriptor logical blocks,
> are not reset when buffer headers are allocated for them. As the
> result, these logical blocks can be treated as free and
> be used for other blocks.This can cause usage of one buffer header
> for several types of data. UDF issues WARNING in this situation:
>
> WARNING: CPU: 0 PID: 2703 at fs/udf/inode.c:2014
> __udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
>
> RIP: 0010:__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
> Call Trace:
> udf_setup_indirect_aext+0x573/0x880 fs/udf/inode.c:1980
> udf_add_aext+0x208/0x2e0 fs/udf/inode.c:2067
> udf_insert_aext fs/udf/inode.c:2233 [inline]
> udf_update_extents fs/udf/inode.c:1181 [inline]
> inode_getblk+0x1981/0x3b70 fs/udf/inode.c:885
>
> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Vladislav Efanov <[email protected]>
> ---
> v2: Do not clear bits related to Bitmap Descriptor logical blocks,
> but return -EFSCORRUPTED error instead.
> fs/udf/balloc.c | 24 ++++++++++++++++++++++++
> fs/udf/udfdecl.h | 1 +
> 2 files changed, 25 insertions(+)
Thanks for the fix!
> bitmap->s_block_bitmap[bitmap_nr] = bh;
> + /* Check consistency of Space Bitmap buffer. */
> + if (bh) {
> + max_bits_others = sb->s_blocksize * 8;
> + max_bits_1st = max_bits_others - (sizeof(struct spaceBitmapDesc) << 3);
> + rest_bits = (bitmap->s_nr_groups > max_bits_1st) ?
> + bitmap->s_nr_groups - max_bits_1st : 0;
> + if (!bitmap_nr)
> + max_bits = min(max_bits_1st, bitmap->s_nr_groups);
> + else if (bitmap_nr < rest_bits / max_bits_others + 1)
> + max_bits = max_bits_others;
So this should be using DIV_ROUND_UP() instead of plain division and + 1
AFAICT. Anyway, I've somewhat simplified these conditions to make things a
bit more obvious and applied your patch. The result is attached for your
reference.
> + else if (bitmap_nr == rest_bits / max_bits_others + 1)
> + max_bits = rest_bits % max_bits_others;
> + for (i = 0; i < max_bits; i++) {
> + if (udf_test_bit(i + (bitmap_nr ? 0 :
> + (sizeof(struct spaceBitmapDesc) << 3)),
> + bh->b_data))
> + return -EFSCORRUPTED;
> + }
> + }
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR