linux-kernel - Re: [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size

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: <20210928175845.ytqj4o4m2e5bsfrr@kari-VirtualBox>
Date:   Tue, 28 Sep 2021 20:58:45 +0300
From:   Kari Argillander <kari.argillander@...il.com>
To:     Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
Cc:     ntfs3@...ts.linux.dev, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size <
 media sector size

On Tue, Sep 28, 2021 at 08:21:54PM +0300, Konstantin Komarov wrote:
> 
> 
> On 27.09.2021 21:56, Kari Argillander wrote:
> > On Mon, Sep 27, 2021 at 06:48:00PM +0300, Konstantin Komarov wrote:
> >> If we continue to work in this case, then we can corrupt fs.
> >>
> > 
> > Should have fixes tag.
> > 
> 
> The bug is in initial commit.
> Do I need to write
> Fixes: 82cae269cfa95 "fs/ntfs3: Add initialization of super block"
> ?

Yes, but format is not right. It needs to be 12 letter sha and need
brackets.

Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")

Just add these to your gitconfig

[core]
	abbrev = 12
[pretty]
	fixes = Fixes: %h (\"%s\")

And after that you can use
	git show -s --pretty=fixes <sha>

You can see also that others have used this same fixes line in commits:

b8155e95de38 ("fs/ntfs3: Fix error handling in indx_insert_into_root()")
8c83a4851da1 ("fs/ntfs3: Potential NULL dereference in hdr_find_split()")
04810f000afd ("fs/ntfs3: Fix error code in indx_add_allocate()")
1263eddfea99 ("fs/ntfs3: Remove unused including <linux/version.h>")
8c01308b6d6b ("fs/ntfs3: Remove unused variable cnt in ntfs_security_init()")
71eeb6ace80b ("fs/ntfs3: Fix integer overflow in multiplication")

The reason for fixes tag is that automatic tools can more easily
cherry-pick things. Kernel stable branches example big these
automatically, but there is also other companys which big these. Also it
is sometimes nice to check commit which introduce this bug. Also some
organization or study might use these for some statics.

> 
> >> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
> >> ---
> >>  fs/ntfs3/super.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> >> index 7099d9b1f3aa..193f9a98f6ab 100644
> >> --- a/fs/ntfs3/super.c
> >> +++ b/fs/ntfs3/super.c
> >> @@ -763,9 +763,14 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
> >>  	sbi->mft.lbo = mlcn << sbi->cluster_bits;
> >>  	sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits;
> >>  
> >> +	/* Compare boot's cluster and sector. */
> > 
> > Pretty random obvious comment and I do not know what this does in this
> > patch.
> > 
> >>  	if (sbi->cluster_size < sbi->sector_size)
> >>  		goto out;
> >>  
> >> +	/* Compare boot's cluster and media sector. */
> >> +	if (sbi->cluster_size < sector_size)
> >> +		goto out; /* No way to use ntfs_get_block in this case. */
> > 
> > Usually comment should not go after line. If you take chunk from patch
> > 3/3 then this is not issue.
> > 
> >> +
> >>  	sbi->cluster_mask = sbi->cluster_size - 1;
> >>  	sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask;
> >>  	sbi->record_size = record_size = boot->record_size < 0
> >> -- 
> >> 2.33.0
> >>
> >>
> >>