linux-kernel - Re: [PATCH] ntfs: Ensure $Extend is a directory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220724153448.GA2608@Negi>
Date:   Sun, 24 Jul 2022 08:34:48 -0700
From:   Soumya Negi <soumya.negi97@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Anton Altaparmakov <anton@...era.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        linux-ntfs-dev@...ts.sourceforge.net,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ntfs: Ensure $Extend is a directory

On Sun, Jul 24, 2022 at 03:47:01PM +0200, Greg KH wrote:
> On Sun, Jul 24, 2022 at 06:21:07AM -0700, Soumya Negi wrote:
> > Fixes Syzbot bug: kernel BUG in ntfs_lookup_inode_by_name
> > https://syzkaller.appspot.com/bug?id=32cf53b48c1846ffc25a185a2e92e170d1a95d71
> > 
> > Check whether $Extend is a directory or not( for NTFS3.0+) while loading
> > system files. If it isn't(as in the case of this bug where the mft record for
> > $Extend contains a regular file), load_system_files() returns false.
> 
> Please wrap your changelog text at 72 columns like your editor asked you
> to when writing this :)

I will correct the changelog(Don't think I can wrap the bug report
link. Checkpatch will still give a warning. Is that okay?).
 
> > Reported-by: syzbot+30b7f850c6d98ea461d2@...kaller.appspotmail.com
> > Signed-off-by: Soumya Negi <soumya.negi97@...il.com>
> 
> What commit caused this problem?  What Fixes: tag should go here?

I don't think this was caused by any specific commit.The $Extend
directory check is not present in any previous releases. Syzbot has
also not been able to produce a cause bisection for the bug. So no fixes
tag(please correct me if I am wrong).

> Should it go to stable kernels?  If so, how far back?

Since the NTFS extension file was new to NTFS 3.0, perhaps the patch 
should apply all the way back to the first release with NTFS3.0 support?
I checked the stable tree history and 2.6.11 is the oldest release I could find.

> > ---
> >  fs/ntfs/super.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> > index 5ae8de09b271..18e2902531f9 100644
> > --- a/fs/ntfs/super.c
> > +++ b/fs/ntfs/super.c
> > @@ -2092,10 +2092,15 @@ static bool load_system_files(ntfs_volume *vol)
> >  	// TODO: Initialize security.
> >  	/* Get the extended system files' directory inode. */
> >  	vol->extend_ino = ntfs_iget(sb, FILE_Extend);
> > -	if (IS_ERR(vol->extend_ino) || is_bad_inode(vol->extend_ino)) {
> > +	if (IS_ERR(vol->extend_ino) || is_bad_inode(vol->extend_ino) ||
> > +	    !S_ISDIR(vol->extend_ino->i_mode)) {
> > +		static const char *es1 = "$Extend is not a directory";
> > +		static const char *es2 = "Failed to load $Extend";
> > +		const char *es = !S_ISDIR(vol->extend_ino->i_mode) ? es1 : es2;
> > +
> >  		if (!IS_ERR(vol->extend_ino))
> >  			iput(vol->extend_ino);
> > -		ntfs_error(sb, "Failed to load $Extend.");
> > +		ntfs_error(sb, "%s.", es);
> 
> Are you allowing the system log to be spammed by an untrusted user with
> this change?

The error message is written to the system log while trying to mount 
the file system(which will fail if the error occurs). I don't understand
how an untrusted user will be involved.

> thanks,
> 
> greg k-h

Thanks,
Soumya