[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 0/8] fs/ntfs3: Bugfix and refactoring

[Konstantin Komarovc]

This series contains various fixes and refactoring for ntfs3.
Added more info into /proc/fs/ntfs3/<dev>/volinfo

Konstantin Komarov (8):
   fs/ntfs3: Use inode_set_ctime_to_ts instead of inode_set_ctime
   fs/ntfs3: Allow repeated call to ntfs3_put_sbi
   fs/ntfs3: Fix alternative boot searching
   fs/ntfs3: Refactoring and comments
   fs/ntfs3: Add more info into /proc/fs/ntfs3/<dev>/volinfo
   fs/ntfs3: Do not allow to change label if volume is read-only
   fs/ntfs3: Fix possible NULL-ptr-deref in ni_readpage_cmpr()
   fs/ntfs3: Fix NULL pointer dereference on error in
     attr_allocate_frame()

  fs/ntfs3/attrib.c  | 12 ++++----
  fs/ntfs3/bitmap.c  |  1 +
  fs/ntfs3/file.c    |  4 +--
  fs/ntfs3/frecord.c |  2 +-
  fs/ntfs3/inode.c   |  5 ++--
  fs/ntfs3/namei.c   |  6 ++--
  fs/ntfs3/ntfs.h    |  2 +-
  fs/ntfs3/ntfs_fs.h |  2 --
  fs/ntfs3/record.c  |  6 ++++
  fs/ntfs3/super.c   | 71 +++++++++++++++++++++++++++++++---------------
  10 files changed, 70 insertions(+), 41 deletions(-)

-- 
2.34.1

[PATCH 1/8] fs/ntfs3: Use inode_set_ctime_to_ts instead of inode_set_ctime

[Konstantin Komarovc]

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

---
  fs/ntfs3/inode.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index eb2ed0701495..2f76dc055c1f 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -170,8 +170,8 @@ static struct inode *ntfs_read_mft(struct inode *inode,
          nt2kernel(std5->cr_time, &ni->i_crtime);
  #endif
          nt2kernel(std5->a_time, &inode->i_atime);
-        ctime = inode_get_ctime(inode);
          nt2kernel(std5->c_time, &ctime);
+        inode_set_ctime_to_ts(inode, ctime);
          nt2kernel(std5->m_time, &inode->i_mtime);

          ni->std_fa = std5->fa;
-- 
2.34.1

[PATCH 2/8] fs/ntfs3: Allow repeated call to ntfs3_put_sbi

[Konstantin Komarovc]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/bitmap.c |  1 +
  fs/ntfs3/super.c  | 21 ++++++++++++++++-----
  2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index d66055e30aff..63f14a0232f6 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -125,6 +125,7 @@ void wnd_close(struct wnd_bitmap *wnd)
      struct rb_node *node, *next;

      kfree(wnd->free_bits);
+    wnd->free_bits = NULL;
      run_close(&wnd->run);

      node = rb_first(&wnd->start_tree);
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index f78c67452b2a..71c80c578feb 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -576,20 +576,30 @@ static noinline void ntfs3_put_sbi(struct 
ntfs_sb_info *sbi)
      wnd_close(&sbi->mft.bitmap);
      wnd_close(&sbi->used.bitmap);

-    if (sbi->mft.ni)
+    if (sbi->mft.ni) {
          iput(&sbi->mft.ni->vfs_inode);
+        sbi->mft.ni = NULL;
+    }

-    if (sbi->security.ni)
+    if (sbi->security.ni) {
          iput(&sbi->security.ni->vfs_inode);
+        sbi->security.ni = NULL;
+    }

-    if (sbi->reparse.ni)
+    if (sbi->reparse.ni) {
          iput(&sbi->reparse.ni->vfs_inode);
+        sbi->reparse.ni = NULL;
+    }

-    if (sbi->objid.ni)
+    if (sbi->objid.ni) {
          iput(&sbi->objid.ni->vfs_inode);
+        sbi->objid.ni = NULL;
+    }

-    if (sbi->volume.ni)
+    if (sbi->volume.ni) {
          iput(&sbi->volume.ni->vfs_inode);
+        sbi->volume.ni = NULL;
+    }

      ntfs_update_mftmirr(sbi, 0);

@@ -1577,6 +1587,7 @@ static int ntfs_fill_super(struct super_block *sb, 
struct fs_context *fc)
      iput(inode);
  out:
      kfree(boot2);
+    ntfs3_put_sbi(sbi);
      return err;
  }

-- 
2.34.1

[PATCH 3/8] fs/ntfs3: Fix alternative boot searching

[Konstantin Komarovc]

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

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 71c80c578feb..d2951b23f52a 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -846,7 +846,7 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,
      struct ntfs_sb_info *sbi = sb->s_fs_info;
      int err;
      u32 mb, gb, boot_sector_size, sct_per_clst, record_size;
-    u64 sectors, clusters, mlcn, mlcn2;
+    u64 sectors, clusters, mlcn, mlcn2, dev_size0;
      struct NTFS_BOOT *boot;
      struct buffer_head *bh;
      struct MFT_REC *rec;
@@ -855,6 +855,9 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,
      u32 boot_off = 0;
      const char *hint = "Primary boot";

+    /* Save original dev_size. Used with alternative boot. */
+    dev_size0 = dev_size;
+
      sbi->volume.blocks = dev_size >> PAGE_SHIFT;

      bh = ntfs_bread(sb, 0);
@@ -1087,9 +1090,9 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,
      }

  out:
-    if (err == -EINVAL && !bh->b_blocknr && dev_size > PAGE_SHIFT) {
+    if (err == -EINVAL && !bh->b_blocknr && dev_size0 > PAGE_SHIFT) {
          u32 block_size = min_t(u32, sector_size, PAGE_SIZE);
-        u64 lbo = dev_size - sizeof(*boot);
+        u64 lbo = dev_size0 - sizeof(*boot);

          /*
            * Try alternative boot (last sector)
@@ -1103,6 +1106,7 @@ static int ntfs_init_from_boot(struct super_block 
*sb, u32 sector_size,

          boot_off = lbo & (block_size - 1);
          hint = "Alternative boot";
+        dev_size = dev_size0; /* restore original size. */
          goto check_boot;
      }
      brelse(bh);
-- 
2.34.1

[PATCH 4/8] fs/ntfs3: Refactoring and comments

[Konstantin Komarovc]

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  |  6 +++---
  fs/ntfs3/file.c    |  4 ++--
  fs/ntfs3/inode.c   |  3 ++-
  fs/ntfs3/namei.c   |  6 +++---
  fs/ntfs3/ntfs.h    |  2 +-
  fs/ntfs3/ntfs_fs.h |  2 --
  fs/ntfs3/record.c  |  6 ++++++
  fs/ntfs3/super.c   | 19 ++++++++-----------
  8 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index a9d82bbb4729..e16487764282 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -1106,10 +1106,10 @@ int attr_data_get_block(struct ntfs_inode *ni, 
CLST vcn, CLST clen, CLST *lcn,
          }
      }

-    /*
+    /*
       * The code below may require additional cluster (to extend 
attribute list)
-     * and / or one MFT record
-     * It is too complex to undo operations if -ENOSPC occurs deep inside
+     * and / or one MFT record
+     * It is too complex to undo operations if -ENOSPC occurs deep inside
       * in 'ni_insert_nonresident'.
       * Return in advance -ENOSPC here if there are no free cluster and 
no free MFT.
       */
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 962f12ce6c0a..1f7a194983c5 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -745,8 +745,8 @@ static ssize_t ntfs_file_read_iter(struct kiocb 
*iocb, struct iov_iter *iter)
  }

  static ssize_t ntfs_file_splice_read(struct file *in, loff_t *ppos,
-                     struct pipe_inode_info *pipe,
-                     size_t len, unsigned int flags)
+                     struct pipe_inode_info *pipe, size_t len,
+                     unsigned int flags)
  {
      struct inode *inode = in->f_mapping->host;
      struct ntfs_inode *ni = ntfs_i(inode);
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 2f76dc055c1f..d6d021e19aaa 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1660,7 +1660,8 @@ struct inode *ntfs_create_inode(struct mnt_idmap 
*idmap, struct inode *dir,
      d_instantiate(dentry, inode);

      /* Set original time. inode times (i_ctime) may be changed in 
ntfs_init_acl. */
-    inode->i_atime = inode->i_mtime = inode_set_ctime_to_ts(inode, 
ni->i_crtime);
+    inode->i_atime = inode->i_mtime =
+        inode_set_ctime_to_ts(inode, ni->i_crtime);
      dir->i_mtime = inode_set_ctime_to_ts(dir, ni->i_crtime);

      mark_inode_dirty(dir);
diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index ad430d50bd79..eedacf94edd8 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -156,8 +156,8 @@ static int ntfs_link(struct dentry *ode, struct 
inode *dir, struct dentry *de)
      err = ntfs_link_inode(inode, de);

      if (!err) {
-        dir->i_mtime = inode_set_ctime_to_ts(inode,
-                             inode_set_ctime_current(dir));
+        dir->i_mtime = inode_set_ctime_to_ts(
+            inode, inode_set_ctime_current(dir));
          mark_inode_dirty(inode);
          mark_inode_dirty(dir);
          d_instantiate(de, inode);
@@ -373,7 +373,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/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 788567d71d93..0e6a2777870c 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -497,8 +497,6 @@ int ntfs_getattr(struct mnt_idmap *idmap, const 
struct path *path,
           struct kstat *stat, u32 request_mask, u32 flags);
  int ntfs3_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
            struct iattr *attr);
-void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
-             CLST len);
  int ntfs_file_open(struct inode *inode, struct file *file);
  int ntfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
          __u64 start, __u64 len);
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 02cc91ed8835..53629b1f65e9 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 d2951b23f52a..f9a214367113 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);
@@ -1082,10 +1081,10 @@ static int ntfs_init_from_boot(struct 
super_block *sb, u32 sector_size,

      if (bh->b_blocknr && !sb_rdonly(sb)) {
          /*
-         * Alternative boot is ok but primary is not ok.
-         * Do not update primary boot here 'cause it may be faked boot.
-         * Let ntfs to be mounted and update boot later.
-         */
+          * Alternative boot is ok but primary is not ok.
+          * Do not update primary boot here 'cause it may be faked boot.
+          * Let ntfs to be mounted and update boot later.
+         */
          *boot2 = kmemdup(boot, sizeof(*boot), GFP_NOFS | __GFP_NOWARN);
      }

@@ -1549,9 +1548,9 @@ static int ntfs_fill_super(struct super_block *sb, 
struct fs_context *fc)

      if (boot2) {
          /*
-         * Alternative boot is ok but primary is not ok.
-         * Volume is recognized as NTFS. Update primary boot.
-         */
+          * Alternative boot is ok but primary is not ok.
+          * Volume is recognized as NTFS. Update primary boot.
+         */
          struct buffer_head *bh0 = sb_getblk(sb, 0);
          if (bh0) {
              if (buffer_locked(bh0))
@@ -1785,7 +1784,6 @@ static int __init init_ntfs_fs(void)
      if (IS_ENABLED(CONFIG_NTFS3_LZX_XPRESS))
          pr_info("ntfs3: Read-only LZX/Xpress compression included\n");

-
  #ifdef CONFIG_PROC_FS
      /* Create "/proc/fs/ntfs3" */
      proc_info_root = proc_mkdir("fs/ntfs3", NULL);
@@ -1827,7 +1825,6 @@ static void __exit exit_ntfs_fs(void)
      if (proc_info_root)
          remove_proc_entry("fs/ntfs3", NULL);
  #endif
-
  }

  MODULE_LICENSE("GPL");
-- 
2.34.1

[PATCH 5/8] fs/ntfs3: Add more info into /proc/fs/ntfs3/<dev>/volinfo

[Konstantin Komarovc]

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

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index f9a214367113..5811da7e9d45 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -453,15 +453,23 @@ static struct proc_dir_entry *proc_info_root;
   * ntfs3.1
   * cluster size
   * number of clusters
+ * total number of mft records
+ * number of used mft records ~= number of files + folders
+ * real state of ntfs "dirty"/"clean"
+ * current state of ntfs "dirty"/"clean"
  */
  static int ntfs3_volinfo(struct seq_file *m, void *o)
  {
      struct super_block *sb = m->private;
      struct ntfs_sb_info *sbi = sb->s_fs_info;

-    seq_printf(m, "ntfs%d.%d\n%u\n%zu\n", sbi->volume.major_ver,
-           sbi->volume.minor_ver, sbi->cluster_size,
-           sbi->used.bitmap.nbits);
+    seq_printf(m, "ntfs%d.%d\n%u\n%zu\n\%zu\n%zu\n%s\n%s\n",
+           sbi->volume.major_ver, sbi->volume.minor_ver,
+           sbi->cluster_size, sbi->used.bitmap.nbits,
+           sbi->mft.bitmap.nbits,
+           sbi->mft.bitmap.nbits - wnd_zeroes(&sbi->mft.bitmap),
+           sbi->volume.real_dirty ? "dirty" : "clean",
+           (sbi->volume.flags & VOLUME_FLAG_DIRTY) ? "dirty" : "clean");

      return 0;
  }
-- 
2.34.1

[PATCH 6/8] fs/ntfs3: Do not allow to change label if volume is read-only

[Konstantin Komarovc]

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 5811da7e9d45..cf0a720523f0 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -497,7 +497,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 7/8] fs/ntfs3: Fix possible NULL-ptr-deref in ni_readpage_cmpr()

[Konstantin Komarovc]

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

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index d49fbb22bd5e..dad976a68985 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -2148,7 +2148,7 @@ int ni_readpage_cmpr(struct ntfs_inode *ni, struct 
page *page)

      for (i = 0; i < pages_per_frame; i++) {
          pg = pages[i];
-        if (i == idx)
+        if (i == idx || !pg)
              continue;
          unlock_page(pg);
          put_page(pg);
-- 
2.34.1

[PATCH 8/8] fs/ntfs3: Fix NULL pointer dereference on error in attr_allocate_frame()

[Konstantin Komarovc]

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

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index e16487764282..63f70259edc0 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -1736,10 +1736,8 @@ int attr_allocate_frame(struct ntfs_inode *ni, 
CLST frame, size_t compr_size,
              le_b = NULL;
              attr_b = ni_find_attr(ni, NULL, &le_b, ATTR_DATA, NULL,
                            0, NULL, &mi_b);
-            if (!attr_b) {
-                err = -ENOENT;
-                goto out;
-            }
+            if (!attr_b)
+                return -ENOENT;

              attr = attr_b;
              le = le_b;
-- 
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?