linux-kernel - Re: [BUG] ntfs: possible data races in ntfs_clear_extent

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] [day] [month] [year] [list]

Message-ID: <5073179D-A781-40B4-8866-0288B29AE4D6@tuxera.com>
Date:   Tue, 13 Jun 2023 08:57:57 +0000
From:   Anton Altaparmakov <anton@...era.com>
To:     Tuo Li <islituo@...il.com>
CC:     Namjae Jeon <linkinjeon@...nel.org>,
        "linux-ntfs-dev@...ts.sourceforge.net" 
        <linux-ntfs-dev@...ts.sourceforge.net>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        "baijiaju1990@...look.com" <baijiaju1990@...look.com>
Subject: Re: [BUG] ntfs: possible data races in ntfs_clear_extent_inode()

Hi,

These are all red herrings.  You do not need to lock something when there is no possibility of another process accessing the data structure.  All the functions you are quoting are inode destruction.  By very definition nothing can possibly have a reference on an inode which is in this code path as it only gets called when there are no references left.  The inode is about to be freed so any access to it would be accessing freed memory.

Thus your static analysis tool is giving you false positive because it doesn't understand the context of what is going on.

Best regards,

	Anton

> On 13 Jun 2023, at 05:34, Tuo Li <islituo@...il.com> wrote:
> 
> Hello,
> 
> Our static analysis tool finds some possible data races in the NTFS file
> system in Linux 6.4.0-rc6.
> 
> In most calling contexts, the variable ni->ext.base_ntfs_ino is accessed
> with holding the lock ni->extent_lock. Here is an example:
> 
>   ntfs_extent_mft_record_free() --> Line 2773 in fs/ntfs/mtf.c
>     mutex_lock(&ni->extent_lock); --> Line 2786 in fs/ntfs/mtf.c (Lock ni->extent_lock)
>     base_ni = ni->ext.base_ntfs_ino; --> Line 2787 in fs/ntfs/mft.c (Access ni->ext.base_ntfs_ino)
> 
> However, in the following calling contexts:
> 
>   ntfs_evict_big_inode() --> Line 2247 in fs/ntfs/inode.c
>      ntfs_clear_extent_inode() --> Line 2274 in fs/ntfs/inode.c
>         if (!is_bad_inode(VFS_I(ni->ext.base_ntfs_ino))) --> Line 2224 in fs/ntfs/inode.c (Access ni->ext.base_ntfs_ino)
> 
>   ntfs_evict_big_inode() --> Line 2247 in fs/ntfs/inode.c
>     ni->ext.base_ntfs_ino = NULL; --> Line 2285 in fs/ntfs/inode.c (Access ni->ext.base_ntfs_ino)
> 
> the variable ni->ext.base_ntfs_ino is accessed without holding the lock
> ni->extent_lock, and thus data races can occur.
> 
> I am not quite sure whether these possible data races are real and how to fix them if they are real.
> Any feedback would be appreciated, thanks!
> 
> Reported-by: BassCheck <bass@...a.edu.cn>
> 
> Best wishes,
> Tuo Li

-- 
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer