[PATCH 00/11] Bugfix and refactoring

[PATCH 00/11] Bugfix and refactoring

[Konstantin Komarov]

This series contains various fixes and refactoring for ntfs3.
Fixed problem with incorrect link counting for files with DOS names.

Konstantin Komarov (11):
   fs/ntfs3: Remove max link count info display during driver init
   fs/ntfs3: Missed le32_to_cpu conversion
   fs/ntfs3: Mark volume as dirty if xattr is broken
   fs/ntfs3: Use variable length array instead of fixed size
   fs/ntfs3: Use 64 bit variable to avoid 32 bit overflow
   fs/ntfs3: Redesign ntfs_create_inode to return error code instead of
     inode
   fs/ntfs3: Check 'folio' pointer for NULL
   fs/ntfs3: Always make file nonresident if fallocate (xfstest 438)
   fs/ntfs3: Optimize to store sorted attribute definition table
   fs/ntfs3: Remove cached label from sbi
   fs/ntfs3: Taking DOS names into account during link counting

  fs/ntfs3/attrib.c  | 103 ++++++++++++++++++++++++++++++++
  fs/ntfs3/file.c    |   9 +++
  fs/ntfs3/fslog.c   |   5 +-
  fs/ntfs3/fsntfs.c  |  77 ++++++++++++++++++++----
  fs/ntfs3/inode.c   |  76 ++++++++++++-----------
  fs/ntfs3/namei.c   |  31 +++-------
  fs/ntfs3/ntfs.h    |   6 +-
  fs/ntfs3/ntfs_fs.h |  77 +++++++++++++++---------
  fs/ntfs3/record.c  |  11 +---
  fs/ntfs3/super.c   | 146 +++++++++++++++++++--------------------------
  fs/ntfs3/xattr.c   |  23 ++++---
  11 files changed, 355 insertions(+), 209 deletions(-)

-- 
2.34.1

[PATCH 01/11] fs/ntfs3: Remove max link count info display during driver init

[Konstantin Komarov]

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

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 9df7c20d066f..ac4722011140 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1804,8 +1804,6 @@ static int __init init_ntfs_fs(void)
  {
      int err;

-    pr_info("ntfs3: Max link count %u\n", NTFS_LINK_MAX);
-
      if (IS_ENABLED(CONFIG_NTFS3_FS_POSIX_ACL))
          pr_info("ntfs3: Enabled Linux POSIX ACLs support\n");
      if (IS_ENABLED(CONFIG_NTFS3_64BIT_CLUSTER))
-- 
2.34.1

[PATCH 02/11] fs/ntfs3: Missed le32_to_cpu conversion

[Konstantin Komarov]

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

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 855519713bf7..d9d08823de62 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -517,7 +517,7 @@ static inline bool is_rst_area_valid(const struct 
RESTART_HDR *rhdr)
          seq_bits -= 1;
      }

-    if (seq_bits != ra->seq_num_bits)
+    if (seq_bits != le32_to_cpu(ra->seq_num_bits))
          return false;

      /* The log page data offset and record header length must be 
quad-aligned. */
-- 
2.34.1

[PATCH 03/11] fs/ntfs3: Mark volume as dirty if xattr is broken

[Konstantin Komarov]

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

diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 53e7d1fa036a..872df2197202 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -200,6 +200,7 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, 
char *buffer,
      int err;
      int ea_size;
      size_t ret;
+    u8 name_len;

      err = ntfs_read_ea(ni, &ea_all, 0, &info);
      if (err)
@@ -215,28 +216,32 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, 
char *buffer,
      for (off = 0; off + sizeof(struct EA_FULL) < size; off += ea_size) {
          ea = Add2Ptr(ea_all, off);
          ea_size = unpacked_ea_size(ea);
+        name_len = ea->name_len;

-        if (!ea->name_len)
+        if (!name_len)
              break;

-        if (ea->name_len > ea_size)
+        if (name_len > ea_size) {
+            ntfs_set_state(ni->mi.sbi, NTFS_DIRTY_ERROR);
+            err = -EINVAL; /* corrupted fs. */
              break;
+        }

          if (buffer) {
              /* Check if we can use field ea->name */
              if (off + ea_size > size)
                  break;

-            if (ret + ea->name_len + 1 > bytes_per_buffer) {
+            if (ret + name_len + 1 > bytes_per_buffer) {
                  err = -ERANGE;
                  goto out;
              }

-            memcpy(buffer + ret, ea->name, ea->name_len);
-            buffer[ret + ea->name_len] = 0;
+            memcpy(buffer + ret, ea->name, name_len);
+            buffer[ret + name_len] = 0;
          }

-        ret += ea->name_len + 1;
+        ret += name_len + 1;
      }

  out:
-- 
2.34.1

[PATCH 04/11] fs/ntfs3: Use variable length array instead of fixed size

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/ntfs.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 9c7478150a03..3d6143c7abc0 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -59,7 +59,7 @@ struct GUID {
  struct cpu_str {
      u8 len;
      u8 unused;
-    u16 name[10];
+    u16 name[];
  };

  struct le_str {
-- 
2.34.1

[PATCH 05/11] fs/ntfs3: Use 64 bit variable to avoid 32 bit overflow

[Konstantin Komarov]

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

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index d9d08823de62..d7807d255dfe 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -1184,7 +1184,8 @@ static int read_log_page(struct ntfs_log *log, u32 
vbo,
  static int log_read_rst(struct ntfs_log *log, bool first,
              struct restart_info *info)
  {
-    u32 skip, vbo;
+    u32 skip;
+    u64 vbo;
      struct RESTART_HDR *r_page = NULL;

      /* Determine which restart area we are looking for. */
-- 
2.34.1

[PATCH 06/11] fs/ntfs3: Redesign ntfs_create_inode to return error code instead of inode

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/inode.c   | 22 +++++++++++-----------
  fs/ntfs3/namei.c   | 31 ++++++++-----------------------
  fs/ntfs3/ntfs_fs.h |  9 ++++-----
  3 files changed, 23 insertions(+), 39 deletions(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index eb7a8c9fba01..85a10d4a74c4 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1210,11 +1210,10 @@ ntfs_create_reparse_buffer(struct ntfs_sb_info 
*sbi, const char *symname,
   *
   * NOTE: if fnd != NULL (ntfs_atomic_open) then @dir is locked
   */
-struct inode *ntfs_create_inode(struct mnt_idmap *idmap, struct inode *dir,
-                struct dentry *dentry,
-                const struct cpu_str *uni, umode_t mode,
-                dev_t dev, const char *symname, u32 size,
-                struct ntfs_fnd *fnd)
+int ntfs_create_inode(struct mnt_idmap *idmap, struct inode *dir,
+              struct dentry *dentry, const struct cpu_str *uni,
+              umode_t mode, dev_t dev, const char *symname, u32 size,
+              struct ntfs_fnd *fnd)
  {
      int err;
      struct super_block *sb = dir->i_sb;
@@ -1239,6 +1238,9 @@ struct inode *ntfs_create_inode(struct mnt_idmap 
*idmap, struct inode *dir,
      struct REPARSE_DATA_BUFFER *rp = NULL;
      bool rp_inserted = false;

+    /* New file will be resident or non resident. */
+    const bool new_file_resident = 1;
+
      if (!fnd)
          ni_lock_dir(dir_ni);

@@ -1478,7 +1480,7 @@ struct inode *ntfs_create_inode(struct mnt_idmap 
*idmap, struct inode *dir,
          attr->size = cpu_to_le32(SIZEOF_RESIDENT);
          attr->name_off = SIZEOF_RESIDENT_LE;
          attr->res.data_off = SIZEOF_RESIDENT_LE;
-    } else if (S_ISREG(mode)) {
+    } else if (!new_file_resident && S_ISREG(mode)) {
          /*
           * Regular file. Create empty non resident data attribute.
           */
@@ -1715,12 +1717,10 @@ struct inode *ntfs_create_inode(struct mnt_idmap 
*idmap, struct inode *dir,
      if (!fnd)
          ni_unlock(dir_ni);

-    if (err)
-        return ERR_PTR(err);
-
-    unlock_new_inode(inode);
+    if (!err)
+        unlock_new_inode(inode);

-    return inode;
+    return err;
  }

  int ntfs_link_inode(struct inode *inode, struct dentry *dentry)
diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index edb6a7141246..71498421ce60 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -107,12 +107,8 @@ static struct dentry *ntfs_lookup(struct inode 
*dir, struct dentry *dentry,
  static int ntfs_create(struct mnt_idmap *idmap, struct inode *dir,
                 struct dentry *dentry, umode_t mode, bool excl)
  {
-    struct inode *inode;
-
-    inode = ntfs_create_inode(idmap, dir, dentry, NULL, S_IFREG | mode, 0,
-                  NULL, 0, NULL);
-
-    return IS_ERR(inode) ? PTR_ERR(inode) : 0;
+    return ntfs_create_inode(idmap, dir, dentry, NULL, S_IFREG | mode, 0,
+                 NULL, 0, NULL);
  }

  /*
@@ -123,12 +119,8 @@ static int ntfs_create(struct mnt_idmap *idmap, 
struct inode *dir,
  static int ntfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
                struct dentry *dentry, umode_t mode, dev_t rdev)
  {
-    struct inode *inode;
-
-    inode = ntfs_create_inode(idmap, dir, dentry, NULL, mode, rdev, 
NULL, 0,
-                  NULL);
-
-    return IS_ERR(inode) ? PTR_ERR(inode) : 0;
+    return ntfs_create_inode(idmap, dir, dentry, NULL, mode, rdev, NULL, 0,
+                 NULL);
  }

  /*
@@ -200,15 +192,12 @@ static int ntfs_symlink(struct mnt_idmap *idmap, 
struct inode *dir,
              struct dentry *dentry, const char *symname)
  {
      u32 size = strlen(symname);
-    struct inode *inode;

      if (unlikely(ntfs3_forced_shutdown(dir->i_sb)))
          return -EIO;

-    inode = ntfs_create_inode(idmap, dir, dentry, NULL, S_IFLNK | 0777, 0,
-                  symname, size, NULL);
-
-    return IS_ERR(inode) ? PTR_ERR(inode) : 0;
+    return ntfs_create_inode(idmap, dir, dentry, NULL, S_IFLNK | 0777, 0,
+                 symname, size, NULL);
  }

  /*
@@ -217,12 +206,8 @@ static int ntfs_symlink(struct mnt_idmap *idmap, 
struct inode *dir,
  static int ntfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
                struct dentry *dentry, umode_t mode)
  {
-    struct inode *inode;
-
-    inode = ntfs_create_inode(idmap, dir, dentry, NULL, S_IFDIR | mode, 0,
-                  NULL, 0, NULL);
-
-    return IS_ERR(inode) ? PTR_ERR(inode) : 0;
+    return ntfs_create_inode(idmap, dir, dentry, NULL, S_IFDIR | mode, 0,
+                 NULL, 0, NULL);
  }

  /*
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 79356fd29a14..3db6a61f61dc 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -714,11 +714,10 @@ int ntfs_sync_inode(struct inode *inode);
  int ntfs_flush_inodes(struct super_block *sb, struct inode *i1,
                struct inode *i2);
  int inode_write_data(struct inode *inode, const void *data, size_t bytes);
-struct inode *ntfs_create_inode(struct mnt_idmap *idmap, struct inode *dir,
-                struct dentry *dentry,
-                const struct cpu_str *uni, umode_t mode,
-                dev_t dev, const char *symname, u32 size,
-                struct ntfs_fnd *fnd);
+int ntfs_create_inode(struct mnt_idmap *idmap, struct inode *dir,
+              struct dentry *dentry, const struct cpu_str *uni,
+              umode_t mode, dev_t dev, const char *symname, u32 size,
+              struct ntfs_fnd *fnd);
  int ntfs_link_inode(struct inode *inode, struct dentry *dentry);
  int ntfs_unlink_inode(struct inode *dir, const struct dentry *dentry);
  void ntfs_evict_inode(struct inode *inode);
-- 
2.34.1

[PATCH 07/11] fs/ntfs3: Check 'folio' pointer for NULL

[Konstantin Komarov]

It can be NULL if bmap is called.

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

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 85a10d4a74c4..94177c1dd818 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -570,13 +570,18 @@ static noinline int ntfs_get_block_vbo(struct 
inode *inode, u64 vbo,
      clear_buffer_uptodate(bh);

      if (is_resident(ni)) {
-        ni_lock(ni);
-        err = attr_data_read_resident(ni, &folio->page);
-        ni_unlock(ni);
-
-        if (!err)
-            set_buffer_uptodate(bh);
+        bh->b_blocknr = RESIDENT_LCN;
          bh->b_size = block_size;
+        if (!folio) {
+            err = 0;
+        } else {
+            ni_lock(ni);
+            err = attr_data_read_resident(ni, &folio->page);
+            ni_unlock(ni);
+
+            if (!err)
+                set_buffer_uptodate(bh);
+        }
          return err;
      }

-- 
2.34.1

[PATCH 08/11] fs/ntfs3: Always make file nonresident if fallocate (xfstest 438)

[Konstantin Komarov]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  | 32 ++++++++++++++++++++++++++++++++
  fs/ntfs3/file.c    |  9 +++++++++
  fs/ntfs3/ntfs_fs.h |  1 +
  3 files changed, 42 insertions(+)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 7aadf5010999..aedae36b91d0 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -2558,3 +2558,35 @@ int attr_insert_range(struct ntfs_inode *ni, u64 
vbo, u64 bytes)

      goto out;
  }
+
+/*
+ * attr_force_nonresident
+ *
+ * Convert default data attribute into non resident form.
+ */
+int attr_force_nonresident(struct ntfs_inode *ni)
+{
+    int err;
+    struct ATTRIB *attr;
+    struct ATTR_LIST_ENTRY *le = NULL;
+    struct mft_inode *mi;
+
+    attr = ni_find_attr(ni, NULL, &le, ATTR_DATA, NULL, 0, NULL, &mi);
+    if (!attr) {
+        ntfs_bad_inode(&ni->vfs_inode, "no data attribute");
+        return -ENOENT;
+    }
+
+    if (attr->non_res) {
+        /* Already non resident. */
+        return 0;
+    }
+
+    down_write(&ni->file.run_lock);
+    err = attr_make_nonresident(ni, attr, le, mi,
+                    le32_to_cpu(attr->res.data_size),
+                    &ni->file.run, &attr, NULL);
+    up_write(&ni->file.run_lock);
+
+    return err;
+}
\ No newline at end of file
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 5418662c80d8..fce8ea098d60 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -578,6 +578,15 @@ static long ntfs_fallocate(struct file *file, int 
mode, loff_t vbo, loff_t len)
          /* Check new size. */
          u8 cluster_bits = sbi->cluster_bits;

+        /* Be sure file is non resident. */
+        if (is_resident(ni)) {
+            ni_lock(ni);
+            err = attr_force_nonresident(ni);
+            ni_unlock(ni);
+            if (err)
+                goto out;
+        }
+
          /* generic/213: expected -ENOSPC instead of -EFBIG. */
          if (!is_supported_holes) {
              loff_t to_alloc = new_size - inode_get_bytes(inode);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 3db6a61f61dc..00dec0ec5648 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -452,6 +452,7 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST 
frame, size_t compr_size,
  int attr_collapse_range(struct ntfs_inode *ni, u64 vbo, u64 bytes);
  int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes);
  int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 
*frame_size);
+int attr_force_nonresident(struct ntfs_inode *ni);

  /* Functions from attrlist.c */
  void al_destroy(struct ntfs_inode *ni);
-- 
2.34.1

[PATCH 09/11] fs/ntfs3: Optimize to store sorted attribute definition table

[Konstantin Komarov]

0x18 bytes instead of 0xa0

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/fsntfs.c  | 60 ++++++++++++++++++++++++++++-
  fs/ntfs3/inode.c   | 30 +++++++--------
  fs/ntfs3/ntfs.h    |  4 --
  fs/ntfs3/ntfs_fs.h | 40 ++++++++++---------
  fs/ntfs3/super.c   | 95 +++++++++++++++-------------------------------
  fs/ntfs3/xattr.c   |  6 +--
  6 files changed, 126 insertions(+), 109 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index ae2ef5c11868..f9c60f3cadaf 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -2698,4 +2698,62 @@ int ntfs_set_label(struct ntfs_sb_info *sbi, u8 
*label, int len)
  out:
      __putname(uni);
      return err;
-}
\ No newline at end of file
+}
+
+/*
+ * Check $AttrDef content and store sorted small $AttrDef entries
+ */
+int ntfs_check_attr_def(struct ntfs_sb_info *sbi,
+            const struct ATTR_DEF_ENTRY *raw, u32 bytes)
+{
+    const struct ATTR_DEF_ENTRY *de_s;
+    struct ATTR_DEF_ENTRY_SMALL *de_d;
+    u32 i, j;
+    u32 max_attr_type;
+    u32 n = (bytes / sizeof(*raw)) * sizeof(*raw);
+
+    for (i = 0, max_attr_type = 0, de_s = raw; i < n; i++, de_s++) {
+        u64 sz;
+        u32 attr_type = le32_to_cpu(de_s->type);
+
+        if (!attr_type)
+            break;
+
+        if ((attr_type & 0xf) || (!i && ATTR_STD != de_s->type) ||
+            (i && le32_to_cpu(de_s[-1].type) >= attr_type)) {
+            return -EINVAL;
+        }
+
+        max_attr_type = attr_type;
+
+        sz = le64_to_cpu(de_s->max_sz);
+        if (de_s->type == ATTR_REPARSE)
+            sbi->attrdef.rp_max_size = sz;
+        else if (de_s->type == ATTR_EA)
+            sbi->attrdef.ea_max_size = sz;
+        else if (de_s->type == ATTR_LABEL)
+            sbi->attrdef.label_max_size = sz;
+    }
+
+    /* Last known attribute type is 0x100. */
+    if (!max_attr_type || max_attr_type > 0x200)
+        return -EINVAL;
+
+    n = max_attr_type >> 4;
+    sbi->attrdef.table = kcalloc(n, sizeof(*de_d), GFP_KERNEL);
+    if (!sbi->attrdef.table)
+        return -ENOMEM;
+
+    for (j = 0, de_s = raw; j < i; j++, de_s++) {
+        u32 idx = (le32_to_cpu(de_s->type) >> 4) - 1;
+        de_d = sbi->attrdef.table + idx;
+
+        de_d->type = de_s->type;
+        de_d->flags = de_s->flags;
+        de_d->min_sz = le64_to_cpu(de_s->min_sz);
+        de_d->max_sz = le64_to_cpu(de_s->max_sz);
+    }
+    sbi->attrdef.entries = n;
+
+    return 0;
+}
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 94177c1dd818..ae4465bf099f 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -18,7 +18,7 @@
  #include "ntfs_fs.h"

  /*
- * ntfs_read_mft - Read record and parses MFT.
+ * ntfs_read_mft - Read record and parse MFT.
   */
  static struct inode *ntfs_read_mft(struct inode *inode,
                     const struct cpu_str *name,
@@ -1090,29 +1090,27 @@ int ntfs_flush_inodes(struct super_block *sb, 
struct inode *i1,
      return ret;
  }

-int inode_write_data(struct inode *inode, const void *data, size_t bytes)
+/*
+ * Helper function to read file.
+ */
+int inode_read_data(struct inode *inode, void *data, size_t bytes)
  {
      pgoff_t idx;
+    struct address_space *mapping = inode->i_mapping;

-    /* Write non resident data. */
      for (idx = 0; bytes; idx++) {
          size_t op = bytes > PAGE_SIZE ? PAGE_SIZE : bytes;
-        struct page *page = ntfs_map_page(inode->i_mapping, idx);
+        struct page *page = read_mapping_page(mapping, idx, NULL);
+        void *kaddr;

          if (IS_ERR(page))
              return PTR_ERR(page);

-        lock_page(page);
-        WARN_ON(!PageUptodate(page));
-        ClearPageUptodate(page);
-
-        memcpy(page_address(page), data, op);
-
-        flush_dcache_page(page);
-        SetPageUptodate(page);
-        unlock_page(page);
+        kaddr = kmap_atomic(page);
+        memcpy(data, kaddr, op);
+        kunmap_atomic(kaddr);

-        ntfs_unmap_page(page);
+        put_page(page);

          bytes -= op;
          data = Add2Ptr(data, PAGE_SIZE);
@@ -1160,7 +1158,7 @@ ntfs_create_reparse_buffer(struct ntfs_sb_info 
*sbi, const char *symname,
      /* err = the length of unicode name of symlink. */
      *nsize = ntfs_reparse_bytes(err);

-    if (*nsize > sbi->reparse.max_size) {
+    if (*nsize > sbi->attrdef.rp_max_size) {
          err = -EFBIG;
          goto out;
      }
@@ -1954,7 +1952,7 @@ static noinline int ntfs_readlink_hlp(const struct 
dentry *link_de,
          rp = NULL;
      }

-    if (size > sbi->reparse.max_size || size <= sizeof(u32))
+    if (size > sbi->attrdef.rp_max_size || size <= sizeof(u32))
          goto out;

      if (!rp) {
diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 3d6143c7abc0..1dd03ba1dc93 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -817,7 +817,6 @@ struct VOLUME_INFO {

  #define SIZEOF_ATTRIBUTE_VOLUME_INFO 0xc

-#define NTFS_LABEL_MAX_LENGTH        (0x100 / sizeof(short))
  #define NTFS_ATTR_INDEXABLE        cpu_to_le32(0x00000002)
  #define NTFS_ATTR_DUPALLOWED        cpu_to_le32(0x00000004)
  #define NTFS_ATTR_MUST_BE_INDEXED    cpu_to_le32(0x00000010)
@@ -1002,9 +1001,6 @@ struct REPARSE_POINT {

  static_assert(sizeof(struct REPARSE_POINT) == 0x18);

-/* Maximum allowed size of the reparse data. */
-#define MAXIMUM_REPARSE_DATA_BUFFER_SIZE    (16 * 1024)
-
  /*
   * The value of the following constant needs to satisfy the following
   * conditions:
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 00dec0ec5648..1d4fb6f87dea 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -201,6 +201,15 @@ struct ntfs_index {
      u8 type; // index_mutex_classed
  };

+/* NOT ondisk!. Just a small copy of $AttrDef file entry. */
+struct ATTR_DEF_ENTRY_SMALL {
+    enum ATTR_TYPE type;
+    __le32 flags;
+    u64 min_sz;
+    u64 max_sz;
+};
+static_assert(sizeof(struct ATTR_DEF_ENTRY_SMALL) == 0x18);
+
  /* Minimum MFT zone. */
  #define NTFS_MIN_MFT_ZONE 100
  /* Step to increase the MFT. */
@@ -242,9 +251,13 @@ struct ntfs_sb_info {
      CLST reparse_no;
      CLST usn_jrnl_no;

-    struct ATTR_DEF_ENTRY *def_table; // Attribute definition table.
-    u32 def_entries;
-    u32 ea_max_size;
+    struct {
+        u64 rp_max_size; // 16K
+        u32 entries;
+        u32 ea_max_size;
+        u32 label_max_size;
+        struct ATTR_DEF_ENTRY_SMALL *table; // 'entries'.
+    } attrdef;

      struct MFT_REC *new_rec;

@@ -296,7 +309,6 @@ struct ntfs_sb_info {
      struct {
          struct ntfs_index index_r;
          struct ntfs_inode *ni;
-        u64 max_size; // 16K
      } reparse;

      struct {
@@ -658,6 +670,8 @@ int run_deallocate(struct ntfs_sb_info *sbi, const 
struct runs_tree *run,
             bool trim);
  bool valid_windows_name(struct ntfs_sb_info *sbi, const struct le_str 
*name);
  int ntfs_set_label(struct ntfs_sb_info *sbi, u8 *label, int len);
+int ntfs_check_attr_def(struct ntfs_sb_info *sbi,
+            const struct ATTR_DEF_ENTRY *raw, u32 bytes);

  /* Globals from index.c */
  int indx_used_bit(struct ntfs_index *indx, struct ntfs_inode *ni, 
size_t *bit);
@@ -714,7 +728,7 @@ int ntfs3_write_inode(struct inode *inode, struct 
writeback_control *wbc);
  int ntfs_sync_inode(struct inode *inode);
  int ntfs_flush_inodes(struct super_block *sb, struct inode *i1,
                struct inode *i2);
-int inode_write_data(struct inode *inode, const void *data, size_t bytes);
+int inode_read_data(struct inode *inode, void *data, size_t bytes);
  int ntfs_create_inode(struct mnt_idmap *idmap, struct inode *dir,
                struct dentry *dentry, const struct cpu_str *uni,
                umode_t mode, dev_t dev, const char *symname, u32 size,
@@ -908,22 +922,6 @@ static inline bool ntfs_is_meta_file(struct 
ntfs_sb_info *sbi, CLST rno)
             rno == sbi->usn_jrnl_no;
  }

-static inline void ntfs_unmap_page(struct page *page)
-{
-    kunmap(page);
-    put_page(page);
-}
-
-static inline struct page *ntfs_map_page(struct address_space *mapping,
-                     unsigned long index)
-{
-    struct page *page = read_mapping_page(mapping, index, NULL);
-
-    if (!IS_ERR(page))
-        kmap(page);
-    return page;
-}
-
  static inline size_t wnd_zone_bit(const struct wnd_bitmap *wnd)
  {
      return wnd->zone_bit;
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ac4722011140..8beefbca5769 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -624,7 +624,7 @@ static void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
  {
      kfree(sbi->new_rec);
      kvfree(ntfs_put_shared(sbi->upcase));
-    kvfree(sbi->def_table);
+    kfree(sbi->attrdef.table);
      kfree(sbi->compress.lznt);
  #ifdef CONFIG_NTFS3_LZX_XPRESS
      xpress_free_decompressor(sbi->compress.xpress);
@@ -1157,8 +1157,6 @@ static int ntfs_fill_super(struct super_block *sb, 
struct fs_context *fc)
      CLST vcn, lcn, len;
      struct ATTRIB *attr;
      const struct VOLUME_INFO *info;
-    u32 idx, done, bytes;
-    struct ATTR_DEF_ENTRY *t;
      u16 *shared;
      struct MFT_REF ref;
      bool ro = sb_rdonly(sb);
@@ -1199,7 +1197,7 @@ static int ntfs_fill_super(struct super_block *sb, 
struct fs_context *fc)

      /*
       * Load $Volume. This should be done before $LogFile
-     * 'cause 'sbi->volume.ni' is used 'ntfs_set_state'.
+     * 'cause 'sbi->volume.ni' is used in 'ntfs_set_state'.
       */
      ref.low = cpu_to_le32(MFT_REC_VOL);
      ref.seq = cpu_to_le16(MFT_REC_VOL);
@@ -1422,54 +1420,28 @@ static int ntfs_fill_super(struct super_block 
*sb, struct fs_context *fc)
          goto put_inode_out;
      }

-    bytes = inode->i_size;
-    sbi->def_table = t = kvmalloc(bytes, GFP_KERNEL);
-    if (!t) {
-        err = -ENOMEM;
-        goto put_inode_out;
-    }
-
-    for (done = idx = 0; done < bytes; done += PAGE_SIZE, idx++) {
-        unsigned long tail = bytes - done;
-        struct page *page = ntfs_map_page(inode->i_mapping, idx);
-
-        if (IS_ERR(page)) {
-            err = PTR_ERR(page);
-            ntfs_err(sb, "Failed to read $AttrDef (%d).", err);
+    {
+        u32 bytes = inode->i_size;
+        struct ATTR_DEF_ENTRY *def_table = kmalloc(bytes, GFP_KERNEL);
+        if (!def_table) {
+            err = -ENOMEM;
              goto put_inode_out;
          }
-        memcpy(Add2Ptr(t, done), page_address(page),
-               min(PAGE_SIZE, tail));
-        ntfs_unmap_page(page);

-        if (!idx && ATTR_STD != t->type) {
-            ntfs_err(sb, "$AttrDef is corrupted.");
-            err = -EINVAL;
-            goto put_inode_out;
+        /* Read the entire file. */
+        err = inode_read_data(inode, def_table, bytes);
+        if (err) {
+            ntfs_err(sb, "Failed to read $AttrDef (%d).", err);
+        } else {
+            /* Check content and store sorted array. */
+            err = ntfs_check_attr_def(sbi, def_table, bytes);
+            if (err)
+                ntfs_err(sb, "$AttrDef is corrupted.");
          }
-    }
-
-    t += 1;
-    sbi->def_entries = 1;
-    done = sizeof(struct ATTR_DEF_ENTRY);
-    sbi->reparse.max_size = MAXIMUM_REPARSE_DATA_BUFFER_SIZE;
-    sbi->ea_max_size = 0x10000; /* default formatter value */
-
-    while (done + sizeof(struct ATTR_DEF_ENTRY) <= bytes) {
-        u32 t32 = le32_to_cpu(t->type);
-        u64 sz = le64_to_cpu(t->max_sz);
-
-        if ((t32 & 0xF) || le32_to_cpu(t[-1].type) >= t32)
-            break;
-
-        if (t->type == ATTR_REPARSE)
-            sbi->reparse.max_size = sz;
-        else if (t->type == ATTR_EA)
-            sbi->ea_max_size = sz;

-        done += sizeof(struct ATTR_DEF_ENTRY);
-        t += 1;
-        sbi->def_entries += 1;
+        kfree(def_table);
+        if (err)
+            goto put_inode_out;
      }
      iput(inode);

@@ -1489,27 +1461,22 @@ static int ntfs_fill_super(struct super_block 
*sb, struct fs_context *fc)
          goto put_inode_out;
      }

-    for (idx = 0; idx < (0x10000 * sizeof(short) >> PAGE_SHIFT); idx++) {
-        const __le16 *src;
-        u16 *dst = Add2Ptr(sbi->upcase, idx << PAGE_SHIFT);
-        struct page *page = ntfs_map_page(inode->i_mapping, idx);
-
-        if (IS_ERR(page)) {
-            err = PTR_ERR(page);
-            ntfs_err(sb, "Failed to read $UpCase (%d).", err);
-            goto put_inode_out;
-        }
-
-        src = page_address(page);
+    /* Read the entire file. */
+    err = inode_read_data(inode, sbi->upcase, 0x10000 * sizeof(short));
+    if (err) {
+        ntfs_err(sb, "Failed to read $UpCase (%d).", err);
+        goto put_inode_out;
+    }

  #ifdef __BIG_ENDIAN
-        for (i = 0; i < PAGE_SIZE / sizeof(u16); i++)
+    {
+        const __le16 *src = sbi->upcase;
+        u16 *dst = sbi->upcase;
+
+        for (i = 0; i < 0x10000; i++)
              *dst++ = le16_to_cpu(*src++);
-#else
-        memcpy(dst, src, PAGE_SIZE);
-#endif
-        ntfs_unmap_page(page);
      }
+#endif

      shared = ntfs_set_shared(sbi->upcase, 0x10000 * sizeof(short));
      if (shared && sbi->upcase != shared) {
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 872df2197202..a7f122e51c04 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -99,12 +99,12 @@ static int ntfs_read_ea(struct ntfs_inode *ni, 
struct EA_FULL **ea,

      /* Check Ea limit. */
      size = le32_to_cpu((*info)->size);
-    if (size > sbi->ea_max_size) {
+    if (size > sbi->attrdef.ea_max_size) {
          err = -EFBIG;
          goto out;
      }

-    if (attr_size(attr_ea) > sbi->ea_max_size) {
+    if (attr_size(attr_ea) > sbi->attrdef.ea_max_size) {
          err = -EFBIG;
          goto out;
      }
@@ -430,7 +430,7 @@ static noinline int ntfs_set_ea(struct inode *inode, 
const char *name,
       * 1. Check ea_info.size_pack for overflow.
       * 2. New attribute size must fit value from $AttrDef
       */
-    if (new_pack > 0xffff || size > sbi->ea_max_size) {
+    if (new_pack > 0xffff || size > sbi->attrdef.ea_max_size) {
          ntfs_inode_warn(
              inode,
              "The size of extended attributes must not exceed 64KiB");
-- 
2.34.1

[PATCH 10/11] fs/ntfs3: Remove cached label from sbi

[Konstantin Komarov]

Add more checks using $AttrDef.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  | 71 ++++++++++++++++++++++++++++++++++++++++++++++
  fs/ntfs3/fsntfs.c  | 17 ++++-------
  fs/ntfs3/ntfs_fs.h | 27 +++++++++++++++++-
  fs/ntfs3/super.c   | 49 +++++++++++++++++++-------------
  4 files changed, 132 insertions(+), 32 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index aedae36b91d0..acee4644fd8d 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -228,6 +228,7 @@ int attr_make_nonresident(struct ntfs_inode *ni, 
struct ATTRIB *attr,
                u64 new_size, struct runs_tree *run,
                struct ATTRIB **ins_attr, struct page *page)
  {
+    const struct ATTR_DEF_ENTRY_SMALL *q;
      struct ntfs_sb_info *sbi;
      struct ATTRIB *attr_s;
      struct MFT_REC *rec;
@@ -243,6 +244,22 @@ int attr_make_nonresident(struct ntfs_inode *ni, 
struct ATTRIB *attr,
      }

      sbi = mi->sbi;
+
+    /* Check if we can use nonresident form. */
+    q = ntfs_query_def(sbi, attr->type);
+    if (!q) {
+        ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
+        return -EINVAL;
+    }
+
+    /* Check resident form. */
+    if (q->flags & NTFS_ATTR_MUST_BE_RESIDENT) {
+        ntfs_warn(sbi->sb,
+              "attribute %x is not allowed to be nonresident",
+              le32_to_cpu(attr->type));
+        return -EINVAL;
+    }
+
      rec = mi->mrec;
      attr_s = NULL;
      used = le32_to_cpu(rec->used);
@@ -2589,4 +2606,58 @@ int attr_force_nonresident(struct ntfs_inode *ni)
      up_write(&ni->file.run_lock);

      return err;
+}
+
+/*
+ * Returns true if attribute is ok
+ */
+bool attr_check(const struct ATTRIB *attr, struct ntfs_sb_info *sbi,
+        struct ntfs_inode *ni)
+{
+    u64 size;
+    const char *hint;
+    const struct ATTR_DEF_ENTRY_SMALL *q = ntfs_query_def(sbi, attr->type);
+
+    if (!q) {
+        hint = "unknown";
+        goto out;
+    }
+
+    /* Check resident form. */
+    if ((q->flags & NTFS_ATTR_MUST_BE_RESIDENT) && attr->non_res) {
+        hint = "must be resident";
+        goto out;
+    }
+
+    /* Check name. */
+    if ((q->flags & NTFS_ATTR_MUST_BE_NAMED) && !attr->name_len) {
+        hint = "must be named";
+        goto out;
+    }
+
+    /* Check size. */
+    size = attr_size(attr);
+    if (size < q->min_sz) {
+        hint = "minimum size";
+        goto out;
+    }
+
+    if (size > q->max_sz) {
+        hint = "maximum size";
+        goto out;
+    }
+
+    /* ok. */
+    return true;
+
+out:
+    if (ni)
+        ntfs_inode_err(&ni->vfs_inode, "attribute type=%x, %s",
+                   le32_to_cpu(attr->type), hint);
+    else
+        ntfs_err(sbi->sb, "attribute type=%x, %s",
+             le32_to_cpu(attr->type), hint);
+
+    ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
+    return false;
  }
\ No newline at end of file
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index f9c60f3cadaf..5dacb8301202 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -2650,8 +2650,8 @@ int ntfs_set_label(struct ntfs_sb_info *sbi, u8 
*label, int len)
  {
      int err;
      struct ATTRIB *attr;
+    u32 uni_bytes;
      struct ntfs_inode *ni = sbi->volume.ni;
-    const u8 max_ulen = 0x80; /* TODO: use attrdef to get maximum length */
      /* Allocate PATH_MAX bytes. */
      struct cpu_str *uni = __getname();

@@ -2663,7 +2663,8 @@ int ntfs_set_label(struct ntfs_sb_info *sbi, u8 
*label, int len)
      if (err < 0)
          goto out;

-    if (uni->len > max_ulen) {
+    uni_bytes = uni->len * sizeof(u16);
+    if (uni_bytes > sbi->attrdef.label_max_size) {
          ntfs_warn(sbi->sb, "new label is too long");
          err = -EFBIG;
          goto out;
@@ -2674,19 +2675,13 @@ int ntfs_set_label(struct ntfs_sb_info *sbi, u8 
*label, int len)
      /* Ignore any errors. */
      ni_remove_attr(ni, ATTR_LABEL, NULL, 0, false, NULL);

-    err = ni_insert_resident(ni, uni->len * sizeof(u16), ATTR_LABEL, NULL,
-                 0, &attr, NULL, NULL);
+    err = ni_insert_resident(ni, uni_bytes, ATTR_LABEL, NULL, 0, &attr,
+                 NULL, NULL);
      if (err < 0)
          goto unlock_out;

      /* write new label in on-disk struct. */
-    memcpy(resident_data(attr), uni->name, uni->len * sizeof(u16));
-
-    /* update cached value of current label. */
-    if (len >= ARRAY_SIZE(sbi->volume.label))
-        len = ARRAY_SIZE(sbi->volume.label) - 1;
-    memcpy(sbi->volume.label, label, len);
-    sbi->volume.label[len] = 0;
+    memcpy(resident_data(attr), uni->name, uni_bytes);
      mark_inode_dirty_sync(&ni->vfs_inode);

  unlock_out:
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 1d4fb6f87dea..12c392db5b08 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -293,7 +293,6 @@ struct ntfs_sb_info {
          __le16 flags; // Cached current VOLUME_INFO::flags, 
VOLUME_FLAG_DIRTY.
          u8 major_ver;
          u8 minor_ver;
-        char label[256];
          bool real_dirty; // Real fs state.
      } volume;

@@ -465,6 +464,8 @@ int attr_collapse_range(struct ntfs_inode *ni, u64 
vbo, u64 bytes);
  int attr_insert_range(struct ntfs_inode *ni, u64 vbo, u64 bytes);
  int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 
*frame_size);
  int attr_force_nonresident(struct ntfs_inode *ni);
+bool attr_check(const struct ATTRIB *attr, struct ntfs_sb_info *sbi,
+        struct ntfs_inode *ni);

  /* Functions from attrlist.c */
  void al_destroy(struct ntfs_inode *ni);
@@ -1152,4 +1153,28 @@ static inline void le64_sub_cpu(__le64 *var, u64 val)
      *var = cpu_to_le64(le64_to_cpu(*var) - val);
  }

+/*
+ * Attributes types: 0x10, 0x20, 0x30....
+ * indexes in attribute table:  0, 1, 2...
+ */
+static inline const struct ATTR_DEF_ENTRY_SMALL *
+ntfs_query_def(const struct ntfs_sb_info *sbi, enum ATTR_TYPE type)
+{
+    const struct ATTR_DEF_ENTRY_SMALL *q;
+    u32 idx = (le32_to_cpu(type) >> 4) - 1;
+
+    if (idx >= sbi->attrdef.entries) {
+        /* such attribute is not allowed in this ntfs. */
+        return NULL;
+    }
+
+    q = sbi->attrdef.table + idx;
+    if (!q->type) {
+        /* such attribute is not allowed in this ntfs. */
+        return NULL;
+    }
+
+    return q;
+}
+
  #endif /* _LINUX_NTFS3_NTFS_FS_H */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 8beefbca5769..dae961d2d6f8 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -481,11 +481,39 @@ static int ntfs3_volinfo_open(struct inode *inode, 
struct file *file)
  /* read /proc/fs/ntfs3/<dev>/label */
  static int ntfs3_label_show(struct seq_file *m, void *o)
  {
+    int len;
      struct super_block *sb = m->private;
      struct ntfs_sb_info *sbi = sb->s_fs_info;
+    struct ATTRIB *attr;
+    u8 *label = kmalloc(PAGE_SIZE, GFP_NOFS);
+
+    if (!label)
+        return -ENOMEM;
+
+    attr = ni_find_attr(sbi->volume.ni, NULL, NULL, ATTR_LABEL, NULL, 0,
+                NULL, NULL);
+
+    if (!attr) {
+        /* It is ok if no ATTR_LABEL */
+        label[0] = 0;
+        len = 0;
+    } else if (!attr_check(attr, sbi, sbi->volume.ni)) {
+        len = sprintf(label, "%pg: failed to get label", sb->s_bdev);
+    } else {
+        len = ntfs_utf16_to_nls(sbi, resident_data(attr),
+                    le32_to_cpu(attr->res.data_size) >> 1,
+                    label, PAGE_SIZE);
+        if (len < 0) {
+            label[0] = 0;
+            len = 0;
+        } else if (len >= PAGE_SIZE) {
+            len = PAGE_SIZE - 1;
+        }
+    }

-    seq_printf(m, "%s\n", sbi->volume.label);
+    seq_printf(m, "%.*s\n", len, label);

+    kfree(label);
      return 0;
  }

@@ -1210,25 +1238,6 @@ static int ntfs_fill_super(struct super_block 
*sb, struct fs_context *fc)

      ni = ntfs_i(inode);

-    /* Load and save label (not necessary). */
-    attr = ni_find_attr(ni, NULL, NULL, ATTR_LABEL, NULL, 0, NULL, NULL);
-
-    if (!attr) {
-        /* It is ok if no ATTR_LABEL */
-    } else if (!attr->non_res && !is_attr_ext(attr)) {
-        /* $AttrDef allows labels to be up to 128 symbols. */
-        err = utf16s_to_utf8s(resident_data(attr),
-                      le32_to_cpu(attr->res.data_size) >> 1,
-                      UTF16_LITTLE_ENDIAN, sbi->volume.label,
-                      sizeof(sbi->volume.label));
-        if (err < 0)
-            sbi->volume.label[0] = 0;
-    } else {
-        /* Should we break mounting here? */
-        //err = -EINVAL;
-        //goto put_inode_out;
-    }
-
      attr = ni_find_attr(ni, attr, NULL, ATTR_VOL_INFO, NULL, 0, NULL, 
NULL);
      if (!attr || is_attr_ext(attr) ||
          !(info = resident_data_ex(attr, SIZEOF_ATTRIBUTE_VOLUME_INFO))) {
-- 
2.34.1

Re: [PATCH 10/11] fs/ntfs3: Remove cached label from sbi

[Nathan Chancellor]

Hi Konstantin,

On Wed, Apr 17, 2024 at 04:09:00PM +0300, Konstantin Komarov wrote:
> Add more checks using $AttrDef.
> 
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

<snip>

> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 8beefbca5769..dae961d2d6f8 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -481,11 +481,39 @@ static int ntfs3_volinfo_open(struct inode *inode,
> struct file *file)
>  /* read /proc/fs/ntfs3/<dev>/label */
>  static int ntfs3_label_show(struct seq_file *m, void *o)
>  {
> +    int len;
>      struct super_block *sb = m->private;
>      struct ntfs_sb_info *sbi = sb->s_fs_info;
> +    struct ATTRIB *attr;
> +    u8 *label = kmalloc(PAGE_SIZE, GFP_NOFS);
> +
> +    if (!label)
> +        return -ENOMEM;
> +
> +    attr = ni_find_attr(sbi->volume.ni, NULL, NULL, ATTR_LABEL, NULL, 0,
> +                NULL, NULL);
> +
> +    if (!attr) {
> +        /* It is ok if no ATTR_LABEL */
> +        label[0] = 0;
> +        len = 0;
> +    } else if (!attr_check(attr, sbi, sbi->volume.ni)) {
> +        len = sprintf(label, "%pg: failed to get label", sb->s_bdev);
> +    } else {
> +        len = ntfs_utf16_to_nls(sbi, resident_data(attr),
> +                    le32_to_cpu(attr->res.data_size) >> 1,
> +                    label, PAGE_SIZE);
> +        if (len < 0) {
> +            label[0] = 0;
> +            len = 0;
> +        } else if (len >= PAGE_SIZE) {
> +            len = PAGE_SIZE - 1;
> +        }
> +    }
> 
> -    seq_printf(m, "%s\n", sbi->volume.label);
> +    seq_printf(m, "%.*s\n", len, label);
> 
> +    kfree(label);
>      return 0;
>  }
> 
> @@ -1210,25 +1238,6 @@ static int ntfs_fill_super(struct super_block *sb,
> struct fs_context *fc)
> 
>      ni = ntfs_i(inode);
> 
> -    /* Load and save label (not necessary). */
> -    attr = ni_find_attr(ni, NULL, NULL, ATTR_LABEL, NULL, 0, NULL, NULL);
> -
> -    if (!attr) {
> -        /* It is ok if no ATTR_LABEL */
> -    } else if (!attr->non_res && !is_attr_ext(attr)) {
> -        /* $AttrDef allows labels to be up to 128 symbols. */
> -        err = utf16s_to_utf8s(resident_data(attr),
> -                      le32_to_cpu(attr->res.data_size) >> 1,
> -                      UTF16_LITTLE_ENDIAN, sbi->volume.label,
> -                      sizeof(sbi->volume.label));
> -        if (err < 0)
> -            sbi->volume.label[0] = 0;
> -    } else {
> -        /* Should we break mounting here? */
> -        //err = -EINVAL;
> -        //goto put_inode_out;
> -    }
> -

The attr initialization above causes the use in the ni_find_attr() to be
uninitialized, which results in the following clang warning (or error
when CONFIG_WERROR is enabled):

  fs/ntfs3/super.c:1247:26: error: variable 'attr' is uninitialized when used here [-Werror,-Wuninitialized]
   1247 |         attr = ni_find_attr(ni, attr, NULL, ATTR_VOL_INFO, NULL, 0, NULL, NULL);
        |                                 ^~~~
  fs/ntfs3/super.c:1192:21: note: initialize the variable 'attr' to silence this warning
   1192 |         struct ATTRIB *attr;
        |                            ^
        |                             = NULL
  1 error generated.

Please either fix this quickly (as this isn't the first time an ntfs3
change has broken us for some time in -next for a similar reason [1]) or
remove this series from -next. Given the issues that Johan has brought
up in other comments in this thread, I feel like the latter may be
better, as this series is clearly not ready for Linus (which is one of
-next's general requirements AFAIUI).

>      attr = ni_find_attr(ni, attr, NULL, ATTR_VOL_INFO, NULL, 0, NULL,
> NULL);
>      if (!attr || is_attr_ext(attr) ||
>          !(info = resident_data_ex(attr, SIZEOF_ATTRIBUTE_VOLUME_INFO))) {

[1]: https://github.com/ClangBuiltLinux/linux/issues/1729

Cheers,
Nathan

[PATCH 11/11] fs/ntfs3: Taking DOS names into account during link counting

[Konstantin Komarov]

When counting and checking hard links in an ntfs file record,

   struct MFT_REC {
     struct NTFS_RECORD_HEADER rhdr; // 'FILE'
     __le16 seq;        // 0x10: Sequence number for this record.
 >>  __le16 hard_links;    // 0x12: The number of hard links to record.
     __le16 attr_off;    // 0x14: Offset to attributes.
   ...

the ntfs3 driver ignored short names (DOS names), causing the link count
to be reduced by 1 and messages to be output to dmesg.

For Windows, such a situation is a minor error, meaning chkdsk does not 
report
errors on such a volume, and in the case of using the /f switch, it silently
corrects them, reporting that no errors were found. This does not affect
the consistency of the file system.

Nevertheless, the behavior in the ntfs3 driver is incorrect and
changes the content of the file system. This patch should fix that.

PS: most likely, there has been a confusion of concepts
MFT_REC::hard_links and inode::__i_nlink.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/inode.c  |  7 ++++---
  fs/ntfs3/record.c | 11 ++---------
  2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index ae4465bf099f..f98200b1a4d2 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -37,7 +37,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
      bool is_dir;
      unsigned long ino = inode->i_ino;
      u32 rp_fa = 0, asize, t32;
-    u16 roff, rsize, names = 0;
+    u16 roff, rsize, names = 0, links = 0;
      const struct ATTR_FILE_NAME *fname = NULL;
      const struct INDEX_ROOT *root;
      struct REPARSE_DATA_BUFFER rp; // 0x18 bytes
@@ -200,11 +200,12 @@ static struct inode *ntfs_read_mft(struct inode 
*inode,
              rsize < SIZEOF_ATTRIBUTE_FILENAME)
              goto out;

+        names += 1;
          fname = Add2Ptr(attr, roff);
          if (fname->type == FILE_NAME_DOS)
              goto next_attr;

-        names += 1;
+        links += 1;
          if (name && name->len == fname->name_len &&
              !ntfs_cmp_names_cpu(name, (struct le_str *)&fname->name_len,
                      NULL, false))
@@ -429,7 +430,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
          ni->mi.dirty = true;
      }

-    set_nlink(inode, names);
+    set_nlink(inode, links);

      if (S_ISDIR(mode)) {
          ni->std_fa |= FILE_ATTRIBUTE_DIRECTORY;
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 6aa3a9d44df1..6c76503edc20 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -534,16 +534,9 @@ bool mi_remove_attr(struct ntfs_inode *ni, struct 
mft_inode *mi,
      if (aoff + asize > used)
          return false;

-    if (ni && is_attr_indexed(attr)) {
+    if (ni && is_attr_indexed(attr) && attr->type == ATTR_NAME) {
          u16 links = le16_to_cpu(ni->mi.mrec->hard_links);
-        struct ATTR_FILE_NAME *fname =
-            attr->type != ATTR_NAME ?
-                NULL :
-                resident_data_ex(attr,
-                         SIZEOF_ATTRIBUTE_FILENAME);
-        if (fname && fname->type == FILE_NAME_DOS) {
-            /* Do not decrease links count deleting DOS name. */
-        } else if (!links) {
+        if (!links) {
              /* minor error. Not critical. */
          } else {
              ni->mi.mrec->hard_links = cpu_to_le16(links - 1);
-- 
2.34.1

Re: [PATCH 11/11] fs/ntfs3: Taking DOS names into account during link counting

[Johan Hovold]

On Wed, Apr 17, 2024 at 04:10:59PM +0300, Konstantin Komarov wrote:
> When counting and checking hard links in an ntfs file record,
> 
>    struct MFT_REC {
>      struct NTFS_RECORD_HEADER rhdr; // 'FILE'
>      __le16 seq;        // 0x10: Sequence number for this record.
>  >>  __le16 hard_links;    // 0x12: The number of hard links to record.
>      __le16 attr_off;    // 0x14: Offset to attributes.
>    ...
> 
> the ntfs3 driver ignored short names (DOS names), causing the link count
> to be reduced by 1 and messages to be output to dmesg.

I also reported seeing link counts being reduced by 2:

[   78.307412] ntfs3: nvme0n1p3: ino=34e6, Correct links count -> 1 (3).
[   78.307843] ntfs3: nvme0n1p3: ino=5bb23, Correct links count -> 1 (2).
[   78.308509] ntfs3: nvme0n1p3: ino=5c722, Correct links count -> 1 (2).
[   78.310018] ntfs3: nvme0n1p3: ino=5d761, Correct links count -> 1 (2).
[   78.310717] ntfs3: nvme0n1p3: ino=33d18, Correct links count -> 1 (3).
[   78.311179] ntfs3: nvme0n1p3: ino=5d75b, Correct links count -> 1 (3).
[   78.311605] ntfs3: nvme0n1p3: ino=5c708, Correct links count -> 1 (3).

 - https://lore.kernel.org/all/Zhz_axTjkJ6Aqeys@hovoldconsulting.com/

Are you sure there are not further issues with this code?

> For Windows, such a situation is a minor error, meaning chkdsk does not 
> report
> errors on such a volume, and in the case of using the /f switch, it silently
> corrects them, reporting that no errors were found. This does not affect
> the consistency of the file system.
> 
> Nevertheless, the behavior in the ntfs3 driver is incorrect and
> changes the content of the file system. This patch should fix that.

This patch is white space damaged and does not apply.

> PS: most likely, there has been a confusion of concepts
> MFT_REC::hard_links and inode::__i_nlink.

I'd also expect a Fixes and CC stable tag here.

And as this patch does not seem to depend on the rest of the series it
should go first (along with any other bug fixes).

> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

Johan

Re: [PATCH 11/11] fs/ntfs3: Taking DOS names into account during link counting

[Konstantin Komarov]

On 18.04.2024 09:31, Johan Hovold wrote:
> On Wed, Apr 17, 2024 at 04:10:59PM +0300, Konstantin Komarov wrote:
>> When counting and checking hard links in an ntfs file record,
>>
>>     struct MFT_REC {
>>       struct NTFS_RECORD_HEADER rhdr; // 'FILE'
>>       __le16 seq;        // 0x10: Sequence number for this record.
>>   >>  __le16 hard_links;    // 0x12: The number of hard links to record.
>>       __le16 attr_off;    // 0x14: Offset to attributes.
>>     ...
>>
>> the ntfs3 driver ignored short names (DOS names), causing the link count
>> to be reduced by 1 and messages to be output to dmesg.
> I also reported seeing link counts being reduced by 2:
>
> [   78.307412] ntfs3: nvme0n1p3: ino=34e6, Correct links count -> 1 (3).
> [   78.307843] ntfs3: nvme0n1p3: ino=5bb23, Correct links count -> 1 (2).
> [   78.308509] ntfs3: nvme0n1p3: ino=5c722, Correct links count -> 1 (2).
> [   78.310018] ntfs3: nvme0n1p3: ino=5d761, Correct links count -> 1 (2).
> [   78.310717] ntfs3: nvme0n1p3: ino=33d18, Correct links count -> 1 (3).
> [   78.311179] ntfs3: nvme0n1p3: ino=5d75b, Correct links count -> 1 (3).
> [   78.311605] ntfs3: nvme0n1p3: ino=5c708, Correct links count -> 1 (3).
>
>   - https://lore.kernel.org/all/Zhz_axTjkJ6Aqeys@hovoldconsulting.com/
>
> Are you sure there are not further issues with this code?
>
>> For Windows, such a situation is a minor error, meaning chkdsk does not
>> report
>> errors on such a volume, and in the case of using the /f switch, it silently
>> corrects them, reporting that no errors were found. This does not affect
>> the consistency of the file system.
>>
>> Nevertheless, the behavior in the ntfs3 driver is incorrect and
>> changes the content of the file system. This patch should fix that.
> This patch is white space damaged and does not apply.
>
>> PS: most likely, there has been a confusion of concepts
>> MFT_REC::hard_links and inode::__i_nlink.
> I'd also expect a Fixes and CC stable tag here.
>
> And as this patch does not seem to depend on the rest of the series it
> should go first (along with any other bug fixes).
>
>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> Johan

Hi Johan,

We are in the process of extending the tests for link counting and 
related scenarios.

If I find bugs, I'll reply ASAP.

Thanks for highlighting the bug.

Re: [PATCH 00/11] Bugfix and refactoring

[Johan Hovold]

On Wed, Apr 17, 2024 at 04:03:46PM +0300, Konstantin Komarov wrote:
> This series contains various fixes and refactoring for ntfs3.
> Fixed problem with incorrect link counting for files with DOS names.
>
> Konstantin Komarov (11):
>    fs/ntfs3: Remove max link count info display during driver init
>    fs/ntfs3: Missed le32_to_cpu conversion
>    fs/ntfs3: Mark volume as dirty if xattr is broken
>    fs/ntfs3: Use variable length array instead of fixed size
>    fs/ntfs3: Use 64 bit variable to avoid 32 bit overflow
>    fs/ntfs3: Redesign ntfs_create_inode to return error code instead of
>      inode
>    fs/ntfs3: Check 'folio' pointer for NULL
>    fs/ntfs3: Always make file nonresident if fallocate (xfstest 438)
>    fs/ntfs3: Optimize to store sorted attribute definition table
>    fs/ntfs3: Remove cached label from sbi
>    fs/ntfs3: Taking DOS names into account during link counting

All the patches in this series appear to be white space damaged and
cannot be applied.

Most of the patches are lacking proper commit messages, and the bug
fixes should be clearly marked as such with a Fixes tag and CC-stable
tag where appropriate.

Also don't mix fixes with cleanups and refactoring unless the former
really depends on the latter.

At least move the independent fixes to the front of the series.

Johan

[PATCH 00/11] Bugfix and refactoring

[Konstantin Komarov]

This series contains various fixes and refactoring for ntfs3.
The 'nocase' option now works correctly.
fileattr was added to support chattr.
Legacy ntfs support was redesigned.

Konstantin Komarov (11):
  fs/ntfs3: Fix field-spanning write in INDEX_HDR
  fs/ntfs3: Fix the format of the "nocase" mount option
  fs/ntfs3: Missed error return
  fs/ntfs3: Keep runs for $MFT::$ATTR_DATA and $MFT::$ATTR_BITMAP
  fs/ntfs3: Do copy_to_user out of run_lock
  fs/ntfs3: Check more cases when directory is corrupted
  fs/ntfs3: Minor ntfs_list_ea refactoring
  fs/ntfs3: Use function file_inode to get inode from file
  fs/ntfs3: Redesign legacy ntfs support
  fs/ntfs3: Implement simple fileattr
  fs/ntfs3: Fix formatting, change comments, renaming

 fs/ntfs3/attrib.c  |   9 ++--
 fs/ntfs3/bitmap.c  |   2 +-
 fs/ntfs3/dir.c     |  54 ++++++++++++++---------
 fs/ntfs3/file.c    | 104 +++++++++++++++++++++++++++++++++++++--------
 fs/ntfs3/frecord.c |  75 ++++++++++++++++++++++++++++++--
 fs/ntfs3/fslog.c   |   8 ++--
 fs/ntfs3/index.c   |   4 +-
 fs/ntfs3/inode.c   |  35 +++++++--------
 fs/ntfs3/namei.c   |   6 +--
 fs/ntfs3/ntfs.h    |   9 ++--
 fs/ntfs3/ntfs_fs.h |  10 +++++
 fs/ntfs3/super.c   |   4 +-
 fs/ntfs3/xattr.c   |  25 +++++------
 13 files changed, 253 insertions(+), 92 deletions(-)

-- 
2.34.1

[PATCH 00/11] Bugfix and refactoring

[Konstantin Komarov]

This series collects fixes and cleanups that accumulated in the ntfs3 tree.

The main fix is 11: attr_allocate_frame() kept using a stale attr_b after 
ni_insert_nonresident() moved the base attribute to another MFT record. 
Patches 02, 05, 07 and 08 fix cluster allocation and sparse file size 
handling, and 10 rejects unaligned hole punching on compressed attributes.

Patch 06 adds a 'ugm' mount option controlling whether uid/gid/mode
are stored in the WSL extended attributes on creation. The rest are 
cleanups.

Konstantin Komarov (11):
  fs/ntfs3: show supported options when an unknown one is given
  fs/ntfs3: zero newly allocated clusters when the request is smaller
    than the frame
  fs/ntfs3: drop unused new_size argument from attr_make_nonresident()
  fs/ntfs3: minor code cleanups
  fs/ntfs3: do not force allocation for IOMAP_ZERO on a partial cluster
  fs/ntfs3: add 'ugm' mount option to control storing uid/gid/mode in
    xattrs
  fs/ntfs3: allow a zero cluster count for resident attributes
  fs/ntfs3: reserve a compression unit in maxbytes_sparse
  fs/ntfs3: take ni_lock() once in ntfs_readdir()
  fs/ntfs3: reject unaligned hole punching on compressed files
  fs/ntfs3: fix stale attr_b in attr_allocate_frame()

 fs/ntfs3/attrib.c  | 130 ++++++++++++++++++++++++---------------------
 fs/ntfs3/dir.c     |  38 ++++++-------
 fs/ntfs3/frecord.c |   5 +-
 fs/ntfs3/inode.c   |  18 +++----
 fs/ntfs3/namei.c   |   6 +--
 fs/ntfs3/ntfs_fs.h |   5 +-
 fs/ntfs3/super.c   |  31 ++++++++++-
 fs/ntfs3/xattr.c   |   7 +++
 8 files changed, 139 insertions(+), 101 deletions(-)

-- 
2.43.0