2022-12-30 11:41:53

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 0/5] fs/ntfs3: Refactoring and bugfix

This series contains various fixes and refactoring for ntfs3.

Konstantin Komarov (5):
  fs/ntfs3: Add null pointer checks
  fs/ntfs3: Improved checking of attribute's name length
  fs/ntfs3: Check for extremely large size of $AttrDef
  fs/ntfs3: Restore overflow checking for attr size in mi_enum_attr
  fs/ntfs3: Refactoring of various minor issues

 fs/ntfs3/bitmap.c  |  3 ++-
 fs/ntfs3/frecord.c |  2 +-
 fs/ntfs3/fsntfs.c  | 22 ++++++++++++++--------
 fs/ntfs3/index.c   | 10 ++++++----
 fs/ntfs3/inode.c   |  8 +++++++-
 fs/ntfs3/namei.c   |  2 +-
 fs/ntfs3/ntfs.h    |  3 ---
 fs/ntfs3/record.c  |  5 +++++
 fs/ntfs3/super.c   | 10 +++++++++-
 9 files changed, 45 insertions(+), 20 deletions(-)

--
2.34.1


2022-12-30 11:43:24

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 4/5] fs/ntfs3: Restore overflow checking for attr size in mi_enum_attr

Fixed comment.
Removed explicit initialization for INDEX_ROOT.

Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/index.c  | 7 ++++---
 fs/ntfs3/record.c | 5 +++++
 fs/ntfs3/super.c  | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 8718df791a55..9fefeac5fe7e 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -994,7 +994,7 @@ struct INDEX_ROOT *indx_get_root(struct ntfs_index
*indx, struct ntfs_inode *ni,
     struct ATTR_LIST_ENTRY *le = NULL;
     struct ATTRIB *a;
     const struct INDEX_NAMES *in = &s_index_names[indx->type];
-    struct INDEX_ROOT *root = NULL;
+    struct INDEX_ROOT *root;

     a = ni_find_attr(ni, NULL, &le, ATTR_ROOT, in->name, in->name_len,
NULL,
              mi);
@@ -1007,8 +1007,9 @@ struct INDEX_ROOT *indx_get_root(struct ntfs_index
*indx, struct ntfs_inode *ni,
     root = resident_data_ex(a, sizeof(struct INDEX_ROOT));

     /* length check */
-    if (root && offsetof(struct INDEX_ROOT, ihdr) +
le32_to_cpu(root->ihdr.used) >
-            le32_to_cpu(a->res.data_size)) {
+    if (root &&
+        offsetof(struct INDEX_ROOT, ihdr) + le32_to_cpu(root->ihdr.used) >
+            le32_to_cpu(a->res.data_size)) {
         return NULL;
     }

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index abfe004774c0..0603169ee8a0 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -220,6 +220,11 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi,
struct ATTRIB *attr)
             return NULL;
         }

+        if (off + asize < off) {
+            /* Overflow check. */
+            return NULL;
+        }
+
         attr = Add2Ptr(attr, asize);
         off += asize;
     }
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 0967035146ce..19d0889b131f 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1187,7 +1187,7 @@ static int ntfs_fill_super(struct super_block *sb,
struct fs_context *fc)

     /*
      * Typical $AttrDef contains up to 20 entries.
-     * Check for extremely large size.
+     * Check for extremely large/small size.
      */
     if (inode->i_size < sizeof(struct ATTR_DEF_ENTRY) ||
         inode->i_size > 100 * sizeof(struct ATTR_DEF_ENTRY)) {
--
2.34.1

2022-12-30 11:46:42

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 1/5] fs/ntfs3: Add null pointer checks

Added null pointer checks in function ntfs_security_init.
Also added le32_to_cpu in functions ntfs_security_init and indx_read.

Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/fsntfs.c | 16 ++++++++++------
 fs/ntfs3/index.c  |  3 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 8de861ddec60..1f36e89dcff7 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -1876,10 +1876,12 @@ int ntfs_security_init(struct ntfs_sb_info *sbi)
         goto out;
     }

-    root_sdh = resident_data_ex(attr, sizeof(struct INDEX_ROOT));
-    if (root_sdh->type != ATTR_ZERO ||
+    if(!(root_sdh = resident_data_ex(attr, sizeof(struct INDEX_ROOT))) ||
+        root_sdh->type != ATTR_ZERO ||
         root_sdh->rule != NTFS_COLLATION_TYPE_SECURITY_HASH ||
-        offsetof(struct INDEX_ROOT, ihdr) + root_sdh->ihdr.used >
attr->res.data_size) {
+        offsetof(struct INDEX_ROOT, ihdr) +
+            le32_to_cpu(root_sdh->ihdr.used) >
+            le32_to_cpu(attr->res.data_size)) {
         err = -EINVAL;
         goto out;
     }
@@ -1895,10 +1897,12 @@ int ntfs_security_init(struct ntfs_sb_info *sbi)
         goto out;
     }

-    root_sii = resident_data_ex(attr, sizeof(struct INDEX_ROOT));
-    if (root_sii->type != ATTR_ZERO ||
+    if(!(root_sii = resident_data_ex(attr, sizeof(struct INDEX_ROOT))) ||
+        root_sii->type != ATTR_ZERO ||
         root_sii->rule != NTFS_COLLATION_TYPE_UINT ||
-        offsetof(struct INDEX_ROOT, ihdr) + root_sii->ihdr.used >
attr->res.data_size) {
+        offsetof(struct INDEX_ROOT, ihdr) +
+            le32_to_cpu(root_sii->ihdr.used) >
+            le32_to_cpu(attr->res.data_size)) {
         err = -EINVAL;
         goto out;
     }
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index f716487ec8a0..8718df791a55 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1102,7 +1102,8 @@ int indx_read(struct ntfs_index *indx, struct
ntfs_inode *ni, CLST vbn,
     }

     /* check for index header length */
-    if (offsetof(struct INDEX_BUFFER, ihdr) + ib->ihdr.used > bytes) {
+    if (offsetof(struct INDEX_BUFFER, ihdr) + le32_to_cpu(ib->ihdr.used) >
+        bytes) {
         err = -EINVAL;
         goto out;
     }
--
2.34.1

2022-12-30 12:05:22

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 5/5] fs/ntfs3: Refactoring of various minor issues

Removed unused macro.
Changed null pointer checking.
Fixed inconsistent indenting.

Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/bitmap.c  | 3 ++-
 fs/ntfs3/frecord.c | 2 +-
 fs/ntfs3/fsntfs.c  | 6 ++++--
 fs/ntfs3/namei.c   | 2 +-
 fs/ntfs3/ntfs.h    | 3 ---
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index 723fb64e6531..393c726ef17a 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -658,7 +658,8 @@ int wnd_init(struct wnd_bitmap *wnd, struct
super_block *sb, size_t nbits)
     if (!wnd->bits_last)
         wnd->bits_last = wbits;

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

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 912eeb3d3471..1103d4d9a497 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1645,7 +1645,7 @@ struct ATTR_FILE_NAME *ni_fname_name(struct
ntfs_inode *ni,
 {
     struct ATTRIB *attr = NULL;
     struct ATTR_FILE_NAME *fname;
-       struct le_str *fns;
+    struct le_str *fns;

     if (le)
         *le = NULL;
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 1f36e89dcff7..342938704cfd 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -2599,8 +2599,10 @@ static inline bool is_reserved_name(struct
ntfs_sb_info *sbi,
     if (len == 4 || (len > 4 && le16_to_cpu(name[4]) == '.')) {
         port_digit = le16_to_cpu(name[3]);
         if (port_digit >= '1' && port_digit <= '9')
-            if (!ntfs_cmp_names(name, 3, COM_NAME, 3, upcase, false) ||
-                !ntfs_cmp_names(name, 3, LPT_NAME, 3, upcase, false))
+            if (!ntfs_cmp_names(name, 3, COM_NAME, 3, upcase,
+                        false) ||
+                !ntfs_cmp_names(name, 3, LPT_NAME, 3, upcase,
+                        false))
                 return true;
     }

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index 3db34d5c03dc..53ddea219e37 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -93,7 +93,7 @@ static struct dentry *ntfs_lookup(struct inode *dir,
struct dentry *dentry,
      * If the MFT record of ntfs inode is not a base record,
inode->i_op can be NULL.
      * This causes null pointer dereference in d_splice_alias().
      */
-    if (!IS_ERR(inode) && inode->i_op == NULL) {
+    if (!IS_ERR_OR_NULL(inode) && !inode->i_op) {
         iput(inode);
         inode = ERR_PTR(-EINVAL);
     }
diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 86ea1826d099..90151e56c122 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -435,9 +435,6 @@ static inline u64 attr_svcn(const struct ATTRIB *attr)
     return attr->non_res ? le64_to_cpu(attr->nres.svcn) : 0;
 }

-/* The size of resident attribute by its resident size. */
-#define BYTES_PER_RESIDENT(b) (0x18 + (b))
-
 static_assert(sizeof(struct ATTRIB) == 0x48);
 static_assert(sizeof(((struct ATTRIB *)NULL)->res) == 0x08);
 static_assert(sizeof(((struct ATTRIB *)NULL)->nres) == 0x38);
--
2.34.1