[BUG] ntfs: possible data races in ntfs_clear_extent_inode()

[BUG] ntfs: possible data races in ntfs_clear_extent_inode()

[Tuo Li]

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@buaa.edu.cn>

Best wishes,
Tuo Li

Re: [BUG] ntfs: possible data races in ntfs_clear_extent_inode()

[Anton Altaparmakov]

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@gmail.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@buaa.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