[PATCH 0/8] fs/ntfs3: Bugfix and refactoring

[PATCH 0/8] fs/ntfs3: Bugfix and refactoring

[Konstantin Komarov]

This series contains various fixes and refactoring for ntfs3.
Added more checks in record.


Konstantin Komarov (8):
   fs/ntfs3: Add ckeck in ni_update_parent()
   fs/ntfs3: Write immediately updated ntfs state
   fs/ntfs3: Minor code refactoring and formatting
   fs/ntfs3: Don't allow to change label if volume is read-only
   fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)
   fs/ntfs3: Add more attributes checks in mi_enum_attr()
   fs/ntfs3: fix deadlock in mark_as_free_ex
   fs/ntfs3: Fix shift-out-of-bounds in ntfs_fill_super

  fs/ntfs3/attrlist.c | 15 +++++++--
  fs/ntfs3/bitmap.c   |  3 +-
  fs/ntfs3/frecord.c  |  6 ++++
  fs/ntfs3/fsntfs.c   | 19 +++++-------
  fs/ntfs3/namei.c    |  2 +-
  fs/ntfs3/ntfs.h     |  2 +-
  fs/ntfs3/ntfs_fs.h  |  2 ++
  fs/ntfs3/record.c   | 74 +++++++++++++++++++++++++++++++++++----------
  fs/ntfs3/super.c    | 38 +++++++++++++++++------
  fs/ntfs3/xattr.c    |  3 +-
  10 files changed, 121 insertions(+), 43 deletions(-)

-- 
2.34.1

[PATCH 1/8] fs/ntfs3: Add ckeck in ni_update_parent()

[Konstantin Komarov]

Check simple case when parent inode equals current inode.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/frecord.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 16bd9faa2d28..8f34d6472ddb 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -3208,6 +3208,12 @@ static bool ni_update_parent(struct ntfs_inode 
*ni, struct NTFS_DUP_INFO *dup,
          if (!fname || !memcmp(&fname->dup, dup, sizeof(fname->dup)))
              continue;

+        /* Check simple case when parent inode equals current inode. */
+        if (ino_get(&fname->home) == ni->vfs_inode.i_ino) {
+            ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
+            continue;
+        }
+
          /* ntfs_iget5 may sleep. */
          dir = ntfs_iget5(sb, &fname->home, NULL);
          if (IS_ERR(dir)) {
-- 
2.34.1

[PATCH 2/8] fs/ntfs3: Write immediately updated ntfs state

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/fsntfs.c | 13 +++----------
  1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 33afee0f5559..edb51dc12f65 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -983,18 +983,11 @@ int ntfs_set_state(struct ntfs_sb_info *sbi, enum 
NTFS_DIRTY_FLAGS dirty)
      if (err)
          return err;

-    mark_inode_dirty(&ni->vfs_inode);
+    mark_inode_dirty_sync(&ni->vfs_inode);
      /* verify(!ntfs_update_mftmirr()); */

-    /*
-     * If we used wait=1, sync_inode_metadata waits for the io for the
-     * inode to finish. It hangs when media is removed.
-     * So wait=0 is sent down to sync_inode_metadata
-     * and filemap_fdatawrite is used for the data blocks.
-     */
-    err = sync_inode_metadata(&ni->vfs_inode, 0);
-    if (!err)
-        err = filemap_fdatawrite(ni->vfs_inode.i_mapping);
+    /* write mft record on disk. */
+    err = _ni_write_inode(&ni->vfs_inode, 1);

      return err;
  }
-- 
2.34.1

[PATCH 3/8] fs/ntfs3: Minor code refactoring and formatting

[Konstantin Komarov]

Trim spaces and clang-format.
Add comment for mi_enum_attr.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/namei.c  | 2 +-
  fs/ntfs3/ntfs.h   | 2 +-
  fs/ntfs3/record.c | 6 ++++++
  fs/ntfs3/super.c  | 3 +--
  fs/ntfs3/xattr.c  | 3 ++-
  5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index 70f8c859e0ad..dd38dbf30add 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -376,7 +376,7 @@ static int ntfs_atomic_open(struct inode *dir, 
struct dentry *dentry,

  #ifdef CONFIG_NTFS3_FS_POSIX_ACL
      if (IS_POSIXACL(dir)) {
-        /*
+        /*
           * Load in cache current acl to avoid ni_lock(dir):
           * ntfs_create_inode -> ntfs_init_acl -> posix_acl_create ->
           * ntfs_get_acl -> ntfs_get_acl_ex -> ni_lock
diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 98b76d1b09e7..86aecbb01a92 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -847,7 +847,7 @@ struct OBJECT_ID {
      // Birth Volume Id is the Object Id of the Volume on.
      // which the Object Id was allocated. It never changes.
      struct GUID BirthVolumeId; //0x10:
-
+
      // Birth Object Id is the first Object Id that was
      // ever assigned to this MFT Record. I.e. If the Object Id
      // is changed for some reason, this field will reflect the
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index c12ebffc94da..cae939cb42cf 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -189,6 +189,12 @@ int mi_read(struct mft_inode *mi, bool is_mft)
      return err;
  }

+/*
+ * mi_enum_attr - start/continue attributes enumeration in record.
+ *
+ * NOTE: mi->mrec - memory of size sbi->record_size
+ * here we sure that mi->mrec->total == sbi->record_size (see mi_read)
+ */
  struct ATTRIB *mi_enum_attr(struct mft_inode *mi, struct ATTRIB *attr)
  {
      const struct MFT_REC *rec = mi->mrec;
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 1a02072b6b0e..d24f2da36bb2 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -488,7 +488,6 @@ static ssize_t ntfs3_label_write(struct file *file, 
const char __user *buffer,
  {
      int err;
      struct super_block *sb = pde_data(file_inode(file));
-    struct ntfs_sb_info *sbi = sb->s_fs_info;
      ssize_t ret = count;
      u8 *label = kmalloc(count, GFP_NOFS);

@@ -502,7 +501,7 @@ static ssize_t ntfs3_label_write(struct file *file, 
const char __user *buffer,
      while (ret > 0 && label[ret - 1] == '\n')
          ret -= 1;

-    err = ntfs_set_label(sbi, label, ret);
+    err = ntfs_set_label(sb->s_fs_info, label, ret);

      if (err < 0) {
          ntfs_err(sb, "failed (%d) to write label", err);
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 1a518550c317..c59d6f5a725a 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -211,7 +211,8 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, 
char *buffer,
      size = le32_to_cpu(info->size);

      /* Enumerate all xattrs. */
-    for (ret = 0, off = 0; off + sizeof(struct EA_FULL) < size; off += 
ea_size) {
+    ret = 0;
+    for (off = 0; off + sizeof(struct EA_FULL) < size; off += ea_size) {
          ea = Add2Ptr(ea_all, off);
          ea_size = unpacked_ea_size(ea);

-- 
2.34.1

[PATCH 4/8] fs/ntfs3: Don't allow to change label if volume is read-only

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/super.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index d24f2da36bb2..da739e509269 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -489,7 +489,12 @@ static ssize_t ntfs3_label_write(struct file *file, 
const char __user *buffer,
      int err;
      struct super_block *sb = pde_data(file_inode(file));
      ssize_t ret = count;
-    u8 *label = kmalloc(count, GFP_NOFS);
+    u8 *label;
+
+    if (sb_rdonly(sb))
+        return -EROFS;
+
+    label = kmalloc(count, GFP_NOFS);

      if (!label)
          return -ENOMEM;
-- 
2.34.1

[PATCH 5/8] fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrlist.c | 15 +++++++++++++--
  fs/ntfs3/bitmap.c   |  3 ++-
  fs/ntfs3/super.c    |  2 +-
  3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/attrlist.c b/fs/ntfs3/attrlist.c
index 42631b31adf1..7c01735d1219 100644
--- a/fs/ntfs3/attrlist.c
+++ b/fs/ntfs3/attrlist.c
@@ -52,7 +52,8 @@ int ntfs_load_attr_list(struct ntfs_inode *ni, struct 
ATTRIB *attr)

      if (!attr->non_res) {
          lsize = le32_to_cpu(attr->res.data_size);
-        le = kmalloc(al_aligned(lsize), GFP_NOFS | __GFP_NOWARN);
+        /* attr is resident: lsize < record_size (1K or 4K) */
+        le = kvmalloc(al_aligned(lsize), GFP_KERNEL);
          if (!le) {
              err = -ENOMEM;
              goto out;
@@ -80,7 +81,17 @@ int ntfs_load_attr_list(struct ntfs_inode *ni, struct 
ATTRIB *attr)
          if (err < 0)
              goto out;

-        le = kmalloc(al_aligned(lsize), GFP_NOFS | __GFP_NOWARN);
+        /* attr is nonresident.
+         * The worst case:
+         * 1T (2^40) extremely fragmented file.
+         * cluster = 4K (2^12) => 2^28 fragments
+         * 2^9 fragments per one record => 2^19 records
+         * 2^5 bytes of ATTR_LIST_ENTRY per one record => 2^24 bytes.
+         *
+         * the result is 16M bytes per attribute list.
+         * Use kvmalloc to allocate in range [several Kbytes - dozen 
Mbytes]
+         */
+        le = kvmalloc(al_aligned(lsize), GFP_KERNEL);
          if (!le) {
              err = -ENOMEM;
              goto out;
diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index 107e808e06ea..d66055e30aff 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -659,7 +659,8 @@ int wnd_init(struct wnd_bitmap *wnd, struct 
super_block *sb, size_t nbits)
          wnd->bits_last = wbits;

      wnd->free_bits =
-        kcalloc(wnd->nwnd, sizeof(u16), GFP_NOFS | __GFP_NOWARN);
+        kvmalloc_array(wnd->nwnd, sizeof(u16), GFP_KERNEL | __GFP_ZERO);
+
      if (!wnd->free_bits)
          return -ENOMEM;

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index da739e509269..0034952b9ccd 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1373,7 +1373,7 @@ static int ntfs_fill_super(struct super_block *sb, 
struct fs_context *fc)
      }

      bytes = inode->i_size;
-    sbi->def_table = t = kmalloc(bytes, GFP_NOFS | __GFP_NOWARN);
+    sbi->def_table = t = kvmalloc(bytes, GFP_KERNEL);
      if (!t) {
          err = -ENOMEM;
          goto put_inode_out;
-- 
2.34.1

Re: [PATCH 5/8] fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)

[Matthew Wilcox]

On Mon, Jul 03, 2023 at 11:26:36AM +0400, Konstantin Komarov wrote:
> 
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

So, um, why?  I mean, I know what kvmalloc does, but why do you want to
do it?  Also, this is missing changes from kfree() to kvfree() so if
you do end up falling back to vmalloc, you'll hit a bug in kfree().

> +++ b/fs/ntfs3/attrlist.c
> @@ -52,7 +52,8 @@ int ntfs_load_attr_list(struct ntfs_inode *ni, struct
> ATTRIB *attr)
> 
>      if (!attr->non_res) {
>          lsize = le32_to_cpu(attr->res.data_size);
> -        le = kmalloc(al_aligned(lsize), GFP_NOFS | __GFP_NOWARN);
> +        /* attr is resident: lsize < record_size (1K or 4K) */
> +        le = kvmalloc(al_aligned(lsize), GFP_KERNEL);
>          if (!le) {
>              err = -ENOMEM;
>              goto out;

This one should be paired with a kvfree in al_destroy(), I think.

> diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
> index 107e808e06ea..d66055e30aff 100644
> --- a/fs/ntfs3/bitmap.c
> +++ b/fs/ntfs3/bitmap.c
> @@ -659,7 +659,8 @@ int wnd_init(struct wnd_bitmap *wnd, struct super_block
> *sb, size_t nbits)
>          wnd->bits_last = wbits;
> 
>      wnd->free_bits =
> -        kcalloc(wnd->nwnd, sizeof(u16), GFP_NOFS | __GFP_NOWARN);
> +        kvmalloc_array(wnd->nwnd, sizeof(u16), GFP_KERNEL | __GFP_ZERO);
> +
>      if (!wnd->free_bits)
>          return -ENOMEM;
> 

This one with wnd_close() and of course later in wnd_init().

> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index da739e509269..0034952b9ccd 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -1373,7 +1373,7 @@ static int ntfs_fill_super(struct super_block *sb,
> struct fs_context *fc)
>      }
> 
>      bytes = inode->i_size;
> -    sbi->def_table = t = kmalloc(bytes, GFP_NOFS | __GFP_NOWARN);
> +    sbi->def_table = t = kvmalloc(bytes, GFP_KERNEL);
>      if (!t) {
>          err = -ENOMEM;
>          goto put_inode_out;

And this one with ntfs3_free_sbi()

Re: [PATCH 5/8] fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)

[Tetsuo Handa]

On 2023/12/23 13:46, Matthew Wilcox wrote:
> On Mon, Jul 03, 2023 at 11:26:36AM +0400, Konstantin Komarov wrote:
>>
>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> 
> So, um, why?  I mean, I know what kvmalloc does, but why do you want to
> do it?  Also, this is missing changes from kfree() to kvfree() so if
> you do end up falling back to vmalloc, you'll hit a bug in kfree().

Right. The first thing we should do is to revert commit fc471e39e38f("fs/ntfs3:
Use kvmalloc instead of kmalloc(... __GFP_NOWARN)"), for that commit is horrible.

> 
>> +++ b/fs/ntfs3/attrlist.c
>> @@ -52,7 +52,8 @@ int ntfs_load_attr_list(struct ntfs_inode *ni, struct
>> ATTRIB *attr)
>>
>>      if (!attr->non_res) {
>>          lsize = le32_to_cpu(attr->res.data_size);
>> -        le = kmalloc(al_aligned(lsize), GFP_NOFS | __GFP_NOWARN);
>> +        /* attr is resident: lsize < record_size (1K or 4K) */
>> +        le = kvmalloc(al_aligned(lsize), GFP_KERNEL);

syzbot is reporting that

  /* attr is resident: lsize < record_size (1K or 4K) */

was false at https://syzkaller.appspot.com/bug?extid=f987ceaddc6bcc334cde .
Maybe you can fix this report by adding more validations. Please be
sure to take into account that the filesystem image is corrupted/crafted.

But you can't replace GFP_NOFS with GFP_KERNEL anyway, for syzbot is also
reporting GFP_KERNEL allocation with filesystem lock held
at https://syzkaller.appspot.com/bug?extid=18f543fc90dd1194c616 .

By the way, you might want to add __GFP_RETRY_MAYFAIL when you can't use
GFP_KERNEL, for GFP_NOFS memory allocation request cannot trigger OOM
killer in order to allocate requested amount of memory, leading to
possibility of out-of-memory deadlock. Especially when there is possibility
that kvmalloc() needs to allocate so many pages...
See https://elixir.bootlin.com/linux/v6.7-rc6/source/mm/page_alloc.c#L3458 .

Re: [PATCH 5/8] fs/ntfs3: Use kvmalloc instead of kmalloc(... __GFP_NOWARN)

[Matthew Wilcox]

On Sat, Dec 23, 2023 at 10:33:11PM +0900, Tetsuo Handa wrote:
> But you can't replace GFP_NOFS with GFP_KERNEL anyway, for syzbot is also
> reporting GFP_KERNEL allocation with filesystem lock held
> at https://syzkaller.appspot.com/bug?extid=18f543fc90dd1194c616 .

Well, you _can_.  What _all_ filesystem authors should be doing is 
switching to memalloc_nofs_save/restore.  Generally when taking a lock
that's needed during reclaim.  In this specific case, soemthing like
this:

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 7b6423584eae..432905489a14 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -122,6 +122,7 @@ int mi_read(struct mft_inode *mi, bool is_mft)
 	struct ntfs_inode *mft_ni = sbi->mft.ni;
 	struct runs_tree *run = mft_ni ? &mft_ni->file.run : NULL;
 	struct rw_semaphore *rw_lock = NULL;
+	unsigned int memalloc = memalloc_nofs_save();
 
 	if (is_mounted(sbi)) {
 		if (!is_mft && mft_ni) {
@@ -177,6 +178,7 @@ int mi_read(struct mft_inode *mi, bool is_mft)
 		goto out;
 	}
 
+	memalloc_nofs_restore(memalloc);
 	return 0;
 
 out:
@@ -186,6 +188,7 @@ int mi_read(struct mft_inode *mi, bool is_mft)
 		err = -EINVAL;
 	}
 
+	memalloc_nofs_restore(memalloc);
 	return err;
 }
 

[PATCH 6/8] fs/ntfs3: Add more attributes checks in mi_enum_attr()

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/record.c | 68 ++++++++++++++++++++++++++++++++++++-----------
  1 file changed, 52 insertions(+), 16 deletions(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index cae939cb42cf..53629b1f65e9 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -199,8 +199,9 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)
  {
      const struct MFT_REC *rec = mi->mrec;
      u32 used = le32_to_cpu(rec->used);
-    u32 t32, off, asize;
+    u32 t32, off, asize, prev_type;
      u16 t16;
+    u64 data_size, alloc_size, tot_size;

      if (!attr) {
          u32 total = le32_to_cpu(rec->total);
@@ -219,6 +220,7 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)
          if (!is_rec_inuse(rec))
              return NULL;

+        prev_type = 0;
          attr = Add2Ptr(rec, off);
      } else {
          /* Check if input attr inside record. */
@@ -232,11 +234,11 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)
              return NULL;
          }

-        if (off + asize < off) {
-            /* Overflow check. */
+        /* Overflow check. */
+        if (off + asize < off)
              return NULL;
-        }

+        prev_type = le32_to_cpu(attr->type);
          attr = Add2Ptr(attr, asize);
          off += asize;
      }
@@ -256,7 +258,11 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)

      /* 0x100 is last known attribute for now. */
      t32 = le32_to_cpu(attr->type);
-    if ((t32 & 0xf) || (t32 > 0x100))
+    if (!t32 || (t32 & 0xf) || (t32 > 0x100))
+        return NULL;
+
+    /* attributes in record must be ordered by type */
+    if (t32 < prev_type)
          return NULL;

      /* Check overflow and boundary. */
@@ -265,16 +271,15 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)

      /* Check size of attribute. */
      if (!attr->non_res) {
+        /* Check resident fields. */
          if (asize < SIZEOF_RESIDENT)
              return NULL;

          t16 = le16_to_cpu(attr->res.data_off);
-
          if (t16 > asize)
              return NULL;

-        t32 = le32_to_cpu(attr->res.data_size);
-        if (t16 + t32 > asize)
+        if (t16 + le32_to_cpu(attr->res.data_size) > asize)
              return NULL;

          t32 = sizeof(short) * attr->name_len;
@@ -284,21 +289,52 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, 
struct ATTRIB *attr)
          return attr;
      }

-    /* Check some nonresident fields. */
-    if (attr->name_len &&
-        le16_to_cpu(attr->name_off) + sizeof(short) * attr->name_len >
-            le16_to_cpu(attr->nres.run_off)) {
+    /* Check nonresident fields. */
+    if (attr->non_res != 1)
+        return NULL;
+
+    t16 = le16_to_cpu(attr->nres.run_off);
+    if (t16 > asize)
+        return NULL;
+
+    t32 = sizeof(short) * attr->name_len;
+    if (t32 && le16_to_cpu(attr->name_off) + t32 > t16)
+        return NULL;
+
+    /* Check start/end vcn. */
+    if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
+        return NULL;
+
+    data_size = le64_to_cpu(attr->nres.data_size);
+    if (le64_to_cpu(attr->nres.valid_size) > data_size)
          return NULL;
-    }

-    if (attr->nres.svcn || !is_attr_ext(attr)) {
+    alloc_size = le64_to_cpu(attr->nres.alloc_size);
+    if (data_size > alloc_size)
+        return NULL;
+
+    t32 = mi->sbi->cluster_mask;
+    if (alloc_size & t32)
+        return NULL;
+
+    if (!attr->nres.svcn && is_attr_ext(attr)) {
+        /* First segment of sparse/compressed attribute */
+        if (asize + 8 < SIZEOF_NONRESIDENT_EX)
+            return NULL;
+
+        tot_size = le64_to_cpu(attr->nres.total_size);
+        if (tot_size & t32)
+            return NULL;
+
+        if (tot_size > alloc_size)
+            return NULL;
+    } else {
          if (asize + 8 < SIZEOF_NONRESIDENT)
              return NULL;

          if (attr->nres.c_unit)
              return NULL;
-    } else if (asize + 8 < SIZEOF_NONRESIDENT_EX)
-        return NULL;
+    }

      return attr;
  }
-- 
2.34.1

[PATCH 7/8] fs/ntfs3: fix deadlock in mark_as_free_ex

[Konstantin Komarov]

Reported-by: syzbot+e94d98936a0ed08bde43@syzkaller.appspotmail.com
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/fsntfs.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index edb51dc12f65..fbfe21dbb425 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -2454,10 +2454,12 @@ void mark_as_free_ex(struct ntfs_sb_info *sbi, 
CLST lcn, CLST len, bool trim)
  {
      CLST end, i, zone_len, zlen;
      struct wnd_bitmap *wnd = &sbi->used.bitmap;
+    bool dirty = false;

      down_write_nested(&wnd->rw_lock, BITMAP_MUTEX_CLUSTERS);
      if (!wnd_is_used(wnd, lcn, len)) {
-        ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
+        /* mark volume as dirty out of wnd->rw_lock */
+        dirty = true;

          end = lcn + len;
          len = 0;
@@ -2511,6 +2513,8 @@ void mark_as_free_ex(struct ntfs_sb_info *sbi, 
CLST lcn, CLST len, bool trim)

  out:
      up_write(&wnd->rw_lock);
+    if (dirty)
+        ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
  }

  /*
-- 
2.34.1

[PATCH 8/8] fs/ntfs3: Fix shift-out-of-bounds in ntfs_fill_super

[Konstantin Komarov]

Reported-by: syzbot+478c1bf0e6bf4a8f3a04@syzkaller.appspotmail.com
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/ntfs_fs.h |  2 ++
  fs/ntfs3/super.c   | 26 ++++++++++++++++++++------
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 629403ede6e5..788567d71d93 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -42,9 +42,11 @@ enum utf16_endian;
  #define MINUS_ONE_T            ((size_t)(-1))
  /* Biggest MFT / smallest cluster */
  #define MAXIMUM_BYTES_PER_MFT        4096
+#define MAXIMUM_SHIFT_BYTES_PER_MFT    12
  #define NTFS_BLOCKS_PER_MFT_RECORD    (MAXIMUM_BYTES_PER_MFT / 512)

  #define MAXIMUM_BYTES_PER_INDEX        4096
+#define MAXIMUM_SHIFT_BYTES_PER_INDEX    12
  #define NTFS_BLOCKS_PER_INODE        (MAXIMUM_BYTES_PER_INDEX / 512)

  /* NTFS specific error code when fixup failed. */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 0034952b9ccd..34ebfaa8fbab 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -905,9 +905,17 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,
          goto out;
      }

-    sbi->record_size = record_size =
-        boot->record_size < 0 ? 1 << (-boot->record_size) :
-                    (u32)boot->record_size << cluster_bits;
+    if (boot->record_size >= 0) {
+        record_size = (u32)boot->record_size << cluster_bits;
+    } else if (-boot->record_size <= MAXIMUM_SHIFT_BYTES_PER_MFT) {
+        record_size = 1u << (-boot->record_size);
+    } else {
+        ntfs_err(sb, "%s: invalid record size %d.", hint,
+             boot->record_size);
+        goto out;
+    }
+
+    sbi->record_size = record_size;
      sbi->record_bits = blksize_bits(record_size);
      sbi->attr_size_tr = (5 * record_size >> 4); // ~320 bytes

@@ -924,9 +932,15 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,
          goto out;
      }

-    sbi->index_size = boot->index_size < 0 ?
-                  1u << (-boot->index_size) :
-                  (u32)boot->index_size << cluster_bits;
+    if (boot->index_size >= 0) {
+        sbi->index_size = (u32)boot->index_size << cluster_bits;
+    } else if (-boot->index_size <= MAXIMUM_SHIFT_BYTES_PER_INDEX) {
+        sbi->index_size = 1u << (-boot->index_size);
+    } else {
+        ntfs_err(sb, "%s: invalid index size %d.", hint,
+             boot->index_size);
+        goto out;
+    }

      /* Check index record size. */
      if (sbi->index_size < SECTOR_SIZE || 
!is_power_of_2(sbi->index_size)) {
-- 
2.34.1

Re: [PATCH 0/8] fs/ntfs3: Bugfix and refactoring

[Kent Overstreet]

On Mon, Jul 03, 2023 at 11:23:49AM +0400, Konstantin Komarov wrote:
> This series contains various fixes and refactoring for ntfs3.
> Added more checks in record.

I don't write explanations for all of my commits, but... zero?