* [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr
@ 2022-04-21 20:53 Pavel Skripkin
2022-04-21 20:53 ` [PATCH 2/2] ntfs3: make ntfs_update_mftmirr return void Pavel Skripkin
2022-06-04 11:42 ` [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin
0 siblings, 2 replies; 5+ messages in thread
From: Pavel Skripkin @ 2022-04-21 20:53 UTC (permalink / raw)
To: almaz.alexandrovich, ntfs3, linux-kernel
Cc: Pavel Skripkin, syzbot+c95173762127ad76a824
If ntfs_fill_super() wasn't called then sbi->sb will be equal to NULL.
Code should check this ptr before dereferencing. Syzbot hit this issue
via passing wrong mount param as can be seen from log below
Fail log:
ntfs3: Unknown parameter 'iochvrset'
general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
CPU: 1 PID: 3589 Comm: syz-executor210 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0
...
Call Trace:
<TASK>
put_ntfs+0x1ed/0x2a0 fs/ntfs3/super.c:463
ntfs_fs_free+0x6a/0xe0 fs/ntfs3/super.c:1363
put_fs_context+0x119/0x7a0 fs/fs_context.c:469
do_new_mount+0x2b4/0xad0 fs/namespace.c:3044
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Reported-and-tested-by: syzbot+c95173762127ad76a824@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
fs/ntfs3/fsntfs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 3de5700a9b83..891125ca6848 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -831,10 +831,15 @@ int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait)
{
int err;
struct super_block *sb = sbi->sb;
- u32 blocksize = sb->s_blocksize;
+ u32 blocksize;
sector_t block1, block2;
u32 bytes;
+ if (!sb)
+ return -EINVAL;
+
+ blocksize = sb->s_blocksize;
+
if (!(sbi->flags & NTFS_FLAGS_MFTMIRR))
return 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] ntfs3: make ntfs_update_mftmirr return void 2022-04-21 20:53 [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin @ 2022-04-21 20:53 ` Pavel Skripkin 2022-06-04 11:42 ` [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin 1 sibling, 0 replies; 5+ messages in thread From: Pavel Skripkin @ 2022-04-21 20:53 UTC (permalink / raw) To: almaz.alexandrovich, ntfs3, linux-kernel; +Cc: Pavel Skripkin None of callers check the return value of ntfs_update_mftmirr(), so make it return void to make code simpler. Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- fs/ntfs3/fsntfs.c | 20 +++++++------------- fs/ntfs3/ntfs_fs.h | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c index 891125ca6848..938acb246b58 100644 --- a/fs/ntfs3/fsntfs.c +++ b/fs/ntfs3/fsntfs.c @@ -827,7 +827,7 @@ int ntfs_refresh_zone(struct ntfs_sb_info *sbi) /* * ntfs_update_mftmirr - Update $MFTMirr data. */ -int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait) +void ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait) { int err; struct super_block *sb = sbi->sb; @@ -836,12 +836,12 @@ int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait) u32 bytes; if (!sb) - return -EINVAL; + return; blocksize = sb->s_blocksize; if (!(sbi->flags & NTFS_FLAGS_MFTMIRR)) - return 0; + return; err = 0; bytes = sbi->mft.recs_mirr << sbi->record_bits; @@ -852,16 +852,13 @@ int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait) struct buffer_head *bh1, *bh2; bh1 = sb_bread(sb, block1++); - if (!bh1) { - err = -EIO; - goto out; - } + if (!bh1) + return; bh2 = sb_getblk(sb, block2++); if (!bh2) { put_bh(bh1); - err = -EIO; - goto out; + return; } if (buffer_locked(bh2)) @@ -881,13 +878,10 @@ int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait) put_bh(bh2); if (err) - goto out; + return; } sbi->flags &= ~NTFS_FLAGS_MFTMIRR; - -out: - return err; } /* diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h index fb825059d488..061dafbdcedd 100644 --- a/fs/ntfs3/ntfs_fs.h +++ b/fs/ntfs3/ntfs_fs.h @@ -592,7 +592,7 @@ int ntfs_look_free_mft(struct ntfs_sb_info *sbi, CLST *rno, bool mft, void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno); int ntfs_clear_mft_tail(struct ntfs_sb_info *sbi, size_t from, size_t to); int ntfs_refresh_zone(struct ntfs_sb_info *sbi); -int ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait); +void ntfs_update_mftmirr(struct ntfs_sb_info *sbi, int wait); enum NTFS_DIRTY_FLAGS { NTFS_DIRTY_CLEAR = 0, NTFS_DIRTY_DIRTY = 1, -- 2.35.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr 2022-04-21 20:53 [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin 2022-04-21 20:53 ` [PATCH 2/2] ntfs3: make ntfs_update_mftmirr return void Pavel Skripkin @ 2022-06-04 11:42 ` Pavel Skripkin 2022-06-09 16:42 ` Konstantin Komarov 1 sibling, 1 reply; 5+ messages in thread From: Pavel Skripkin @ 2022-06-04 11:42 UTC (permalink / raw) To: almaz.alexandrovich, ntfs3, linux-kernel; +Cc: syzbot+c95173762127ad76a824 [-- Attachment #1.1: Type: text/plain, Size: 1184 bytes --] On 4/21/22 23:53, Pavel Skripkin wrote: > If ntfs_fill_super() wasn't called then sbi->sb will be equal to NULL. > Code should check this ptr before dereferencing. Syzbot hit this issue > via passing wrong mount param as can be seen from log below > > Fail log: > ntfs3: Unknown parameter 'iochvrset' > general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN > KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f] > CPU: 1 PID: 3589 Comm: syz-executor210 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0 > ... > Call Trace: > <TASK> > put_ntfs+0x1ed/0x2a0 fs/ntfs3/super.c:463 > ntfs_fs_free+0x6a/0xe0 fs/ntfs3/super.c:1363 > put_fs_context+0x119/0x7a0 fs/fs_context.c:469 > do_new_mount+0x2b4/0xad0 fs/namespace.c:3044 > do_mount fs/namespace.c:3383 [inline] > __do_sys_mount fs/namespace.c:3591 [inline] > > Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") > Reported-and-tested-by: syzbot+c95173762127ad76a824@syzkaller.appspotmail.com > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> gentle ping With regards, Pavel Skripkin [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr 2022-06-04 11:42 ` [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin @ 2022-06-09 16:42 ` Konstantin Komarov 2022-07-05 13:34 ` Konstantin Komarov 0 siblings, 1 reply; 5+ messages in thread From: Konstantin Komarov @ 2022-06-09 16:42 UTC (permalink / raw) To: Pavel Skripkin, ntfs3; +Cc: syzbot+c95173762127ad76a824, linux-kernel On 6/4/22 14:42, Pavel Skripkin wrote: > On 4/21/22 23:53, Pavel Skripkin wrote: >> If ntfs_fill_super() wasn't called then sbi->sb will be equal to NULL. >> Code should check this ptr before dereferencing. Syzbot hit this issue >> via passing wrong mount param as can be seen from log below >> >> Fail log: >> ntfs3: Unknown parameter 'iochvrset' >> general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN >> KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f] >> CPU: 1 PID: 3589 Comm: syz-executor210 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0 >> ... >> Call Trace: >> <TASK> >> put_ntfs+0x1ed/0x2a0 fs/ntfs3/super.c:463 >> ntfs_fs_free+0x6a/0xe0 fs/ntfs3/super.c:1363 >> put_fs_context+0x119/0x7a0 fs/fs_context.c:469 >> do_new_mount+0x2b4/0xad0 fs/namespace.c:3044 >> do_mount fs/namespace.c:3383 [inline] >> __do_sys_mount fs/namespace.c:3591 [inline] >> >> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") >> Reported-and-tested-by: syzbot+c95173762127ad76a824@syzkaller.appspotmail.com >> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > > gentle ping > > > > > With regards, > Pavel Skripkin 1st patch is correct. 2nd patch is a good catch, but I'm not sure if simply ignoring is good. If mftmirr is broken / missing, then theoretically we can continue working. But still it's a major fs error. I'm thinking about exiting mount with error, and if "force" is present, then continue with mount. I'll reply again when I'll be sure what is correct behavior. Thank you for your work! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr 2022-06-09 16:42 ` Konstantin Komarov @ 2022-07-05 13:34 ` Konstantin Komarov 0 siblings, 0 replies; 5+ messages in thread From: Konstantin Komarov @ 2022-07-05 13:34 UTC (permalink / raw) To: Pavel Skripkin, ntfs3; +Cc: syzbot+c95173762127ad76a824, linux-kernel On 6/9/22 19:42, Konstantin Komarov wrote: > > > On 6/4/22 14:42, Pavel Skripkin wrote: >> On 4/21/22 23:53, Pavel Skripkin wrote: >>> If ntfs_fill_super() wasn't called then sbi->sb will be equal to NULL. >>> Code should check this ptr before dereferencing. Syzbot hit this issue >>> via passing wrong mount param as can be seen from log below >>> >>> Fail log: >>> ntfs3: Unknown parameter 'iochvrset' >>> general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN >>> KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f] >>> CPU: 1 PID: 3589 Comm: syz-executor210 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0 >>> ... >>> Call Trace: >>> <TASK> >>> put_ntfs+0x1ed/0x2a0 fs/ntfs3/super.c:463 >>> ntfs_fs_free+0x6a/0xe0 fs/ntfs3/super.c:1363 >>> put_fs_context+0x119/0x7a0 fs/fs_context.c:469 >>> do_new_mount+0x2b4/0xad0 fs/namespace.c:3044 >>> do_mount fs/namespace.c:3383 [inline] >>> __do_sys_mount fs/namespace.c:3591 [inline] >>> >>> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") >>> Reported-and-tested-by: syzbot+c95173762127ad76a824@syzkaller.appspotmail.com >>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> >> >> gentle ping >> >> >> >> >> With regards, >> Pavel Skripkin > > 1st patch is correct. > 2nd patch is a good catch, but I'm not sure if simply ignoring is good. > If mftmirr is broken / missing, then theoretically we can continue working. > But still it's a major fs error. > I'm thinking about exiting mount with error, and if "force" is present, > then continue with mount. > I'll reply again when I'll be sure what is correct behavior. > Thank you for your work! I've accepted both patches. I don't want to invent convoluted logic for mftmirr. Thanks again for your work! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-05 13:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-21 20:53 [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin 2022-04-21 20:53 ` [PATCH 2/2] ntfs3: make ntfs_update_mftmirr return void Pavel Skripkin 2022-06-04 11:42 ` [PATCH 1/2] ntfs3: fix NULL deref in ntfs_update_mftmirr Pavel Skripkin 2022-06-09 16:42 ` Konstantin Komarov 2022-07-05 13:34 ` Konstantin Komarov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).