linux-kernel - [PATCH v2 3/3] fs/ntfs3: Refactor ni_try_remove_attr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 30 May 2022 19:40:55 +0300
From:   Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To:     <ntfs3@...ts.linux.dev>
CC:     <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>
Subject: [PATCH v2 3/3] fs/ntfs3: Refactor ni_try_remove_attr_list function

Now we save a copy of primary record for restoration.
Also now we remove all attributes from subrecords.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
---
  fs/ntfs3/frecord.c | 49 ++++++++++++++++++++++++++++++++++------------
  fs/ntfs3/record.c  |  5 ++---
  2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 18842998c8fa..3576268ee0a1 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -7,6 +7,7 @@
  
  #include <linux/fiemap.h>
  #include <linux/fs.h>
+#include <linux/minmax.h>
  #include <linux/vmalloc.h>
  
  #include "debug.h"
@@ -649,6 +650,7 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
  	struct mft_inode *mi;
  	u32 asize, free;
  	struct MFT_REF ref;
+	struct MFT_REC *mrec;
  	__le16 id;
  
  	if (!ni->attr_list.dirty)
@@ -692,11 +694,17 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
  		free -= asize;
  	}
  
+	/* Make a copy of primary record to restore if error. */
+	mrec = kmemdup(ni->mi.mrec, sbi->record_size, GFP_NOFS);
+	if (!mrec)
+		return 0; /* Not critical. */
+
  	/* It seems that attribute list can be removed from primary record. */
  	mi_remove_attr(NULL, &ni->mi, attr_list);
  
  	/*
-	 * Repeat the cycle above and move all attributes to primary record.
+	 * Repeat the cycle above and copy all attributes to primary record.
+	 * Do not remove original attributes from subrecords!
  	 * It should be success!
  	 */
  	le = NULL;
@@ -707,14 +715,14 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
  		mi = ni_find_mi(ni, ino_get(&le->ref));
  		if (!mi) {
  			/* Should never happened, 'cause already checked. */
-			goto bad;
+			goto out;
  		}
  
  		attr = mi_find_attr(mi, NULL, le->type, le_name(le),
  				    le->name_len, &le->id);
  		if (!attr) {
  			/* Should never happened, 'cause already checked. */
-			goto bad;
+			goto out;
  		}
  		asize = le32_to_cpu(attr->size);
  
@@ -724,18 +732,33 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
  					  le16_to_cpu(attr->name_off));
  		if (!attr_ins) {
  			/*
-			 * Internal error.
-			 * Either no space in primary record (already checked).
-			 * Either tried to insert another
-			 * non indexed attribute (logic error).
+			 * No space in primary record (already checked).
  			 */
-			goto bad;
+			goto out;
  		}
  
  		/* Copy all except id. */
  		id = attr_ins->id;
  		memcpy(attr_ins, attr, asize);
  		attr_ins->id = id;
+	}
+
+	/*
+	 * Repeat the cycle above and remove all attributes from subrecords.
+	 */
+	le = NULL;
+	while ((le = al_enumerate(ni, le))) {
+		if (!memcmp(&le->ref, &ref, sizeof(ref)))
+			continue;
+
+		mi = ni_find_mi(ni, ino_get(&le->ref));
+		if (!mi)
+			continue;
+
+		attr = mi_find_attr(mi, NULL, le->type, le_name(le),
+				    le->name_len, &le->id);
+		if (!attr)
+			continue;
  
  		/* Remove from original record. */
  		mi_remove_attr(NULL, mi, attr);
@@ -748,11 +771,13 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
  	ni->attr_list.le = NULL;
  	ni->attr_list.dirty = false;
  
+	kfree(mrec);
+	return 0;
+out:
+	/* Restore primary record. */
+	swap(mrec, ni->mi.mrec);
+	kfree(mrec);
  	return 0;
-bad:
-	ntfs_inode_err(&ni->vfs_inode, "Internal error");
-	make_bad_inode(&ni->vfs_inode);
-	return -EINVAL;
  }
  
  /*
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 861e35791506..8fe0a876400a 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -445,12 +445,11 @@ struct ATTRIB *mi_insert_attr(struct mft_inode *mi, enum ATTR_TYPE type,
  	attr = NULL;
  	while ((attr = mi_enum_attr(mi, attr))) {
  		diff = compare_attr(attr, type, name, name_len, upcase);
-		if (diff > 0)
-			break;
+
  		if (diff < 0)
  			continue;
  
-		if (!is_attr_indexed(attr))
+		if (!diff && !is_attr_indexed(attr))
  			return NULL;
  		break;
  	}
-- 
2.36.1