All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring
@ 2022-10-28 17:00 Konstantin Komarov
  2022-10-28 17:01 ` [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters Konstantin Komarov
                   ` (13 more replies)
  0 siblings, 14 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:00 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Second part of various fixes and refactoring for ntfs3.

Konstantin Komarov (14):
   fs/ntfs3: Fixing work with sparse clusters
   fs/ntfs3: Change new sparse cluster processing
   fs/ntfs3: Fix wrong indentations
   fs/ntfs3: atomic_open implementation
   fs/ntfs3: Fixing wrong logic in attr_set_size and ntfs_fallocate
   fs/ntfs3: Changing locking in ntfs_rename
   fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block
   fs/ntfs3: Correct ntfs_check_for_free_space
   fs/ntfs3: Check fields while reading
   fs/ntfs3: Fix incorrect if in ntfs_set_acl_ex
   fs/ntfs3: Use ALIGN kernel macro
   fs/ntfs3: Fix wrong if in hdr_first_de
   fs/ntfs3: Improve checking of bad clusters
   fs/ntfs3: Make if more readable

  fs/ntfs3/attrib.c  | 338 +++++++++++++++++++++++++++++----------------
  fs/ntfs3/bitmap.c  |  38 +++++
  fs/ntfs3/file.c    | 203 ++++++++-------------------
  fs/ntfs3/frecord.c |   2 +-
  fs/ntfs3/fslog.c   |   3 +-
  fs/ntfs3/fsntfs.c  |  35 ++++-
  fs/ntfs3/index.c   | 105 ++++++++++++--
  fs/ntfs3/inode.c   |  86 +++++++-----
  fs/ntfs3/namei.c   | 104 ++++++++++++++
  fs/ntfs3/ntfs.h    |   6 +-
  fs/ntfs3/ntfs_fs.h |  22 ++-
  fs/ntfs3/record.c  |   5 +-
  fs/ntfs3/run.c     |  28 +---
  fs/ntfs3/super.c   |  64 +++++----
  fs/ntfs3/xattr.c   | 116 ++++++++++------
  15 files changed, 737 insertions(+), 418 deletions(-)

-- 
2.37.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
@ 2022-10-28 17:01 ` Konstantin Komarov
  2022-10-28 17:02 ` [PATCH 02/14] fs/ntfs3: Change new sparse cluster processing Konstantin Komarov
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:01 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Simplify logic in ntfs_extend_initialized_size, ntfs_sparse_cluster
and ntfs_fallocate.

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

diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 4f2ffc7ef296..96ba3f5a8470 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -128,25 +128,9 @@ static int ntfs_extend_initialized_size(struct file *file,
  				goto out;
  
  			if (lcn == SPARSE_LCN) {
-				loff_t vbo = (loff_t)vcn << bits;
-				loff_t to = vbo + ((loff_t)clen << bits);
-
-				if (to <= new_valid) {
-					ni->i_valid = to;
-					pos = to;
-					goto next;
-				}
-
-				if (vbo < pos) {
-					pos = vbo;
-				} else {
-					to = (new_valid >> bits) << bits;
-					if (pos < to) {
-						ni->i_valid = to;
-						pos = to;
-						goto next;
-					}
-				}
+				pos = ((loff_t)clen + vcn) << bits;
+				ni->i_valid = pos;
+				goto next;
  			}
  		}
  
@@ -279,8 +263,9 @@ void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
  {
  	struct address_space *mapping = inode->i_mapping;
  	struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info;
-	u64 vbo = (u64)vcn << sbi->cluster_bits;
-	u64 bytes = (u64)len << sbi->cluster_bits;
+	u8 cluster_bits = sbi->cluster_bits;
+	u64 vbo = (u64)vcn << cluster_bits;
+	u64 bytes = (u64)len << cluster_bits;
  	u32 blocksize = 1 << inode->i_blkbits;
  	pgoff_t idx0 = page0 ? page0->index : -1;
  	loff_t vbo_clst = vbo & sbi->cluster_mask_inv;
@@ -329,11 +314,10 @@ void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
  
  		zero_user_segment(page, from, to);
  
-		if (!partial) {
-			if (!PageUptodate(page))
-				SetPageUptodate(page);
-			set_page_dirty(page);
-		}
+		if (!partial)
+			SetPageUptodate(page);
+		flush_dcache_page(page);
+		set_page_dirty(page);
  
  		if (idx != idx0) {
  			unlock_page(page);
@@ -341,7 +325,6 @@ void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
  		}
  		cond_resched();
  	}
-	mark_inode_dirty(inode);
  }
  
  /*
@@ -588,11 +571,7 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  		u32 frame_size;
  		loff_t mask, vbo_a, end_a, tmp;
  
-		err = filemap_write_and_wait_range(mapping, vbo, end - 1);
-		if (err)
-			goto out;
-
-		err = filemap_write_and_wait_range(mapping, end, LLONG_MAX);
+		err = filemap_write_and_wait_range(mapping, vbo, LLONG_MAX);
  		if (err)
  			goto out;
  
@@ -693,7 +672,7 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  			goto out;
  
  		if (is_supported_holes) {
-			CLST vcn_v = ni->i_valid >> sbi->cluster_bits;
+			CLST vcn_v = bytes_to_cluster(sbi, ni->i_valid);
  			CLST vcn = vbo >> sbi->cluster_bits;
  			CLST cend = bytes_to_cluster(sbi, end);
  			CLST lcn, clen;
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index e9cf00d14733..f487d36c9b78 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -645,7 +645,12 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
  			bh->b_size = block_size;
  			off = vbo & (PAGE_SIZE - 1);
  			set_bh_page(bh, page, off);
-			ll_rw_block(REQ_OP_READ, 1, &bh);
+
+			lock_buffer(bh);
+			bh->b_end_io = end_buffer_read_sync;
+			get_bh(bh);
+			submit_bh(REQ_OP_READ, bh);
+
  			wait_on_buffer(bh);
  			if (!buffer_uptodate(bh)) {
  				err = -EIO;
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/14] fs/ntfs3: Change new sparse cluster processing
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
  2022-10-28 17:01 ` [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters Konstantin Komarov
@ 2022-10-28 17:02 ` Konstantin Komarov
  2022-10-28 17:02 ` [PATCH 03/14] fs/ntfs3: Fix wrong indentations Konstantin Komarov
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:02 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Remove ntfs_sparse_cluster.
Zero clusters in attr_allocate_clusters.
Fixes xfstest generic/263

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  | 176 +++++++++++++++++++++++++++++++--------------
  fs/ntfs3/file.c    | 146 +++++++++----------------------------
  fs/ntfs3/frecord.c |   2 +-
  fs/ntfs3/index.c   |   4 +-
  fs/ntfs3/inode.c   |  12 ++--
  fs/ntfs3/ntfs_fs.h |   7 +-
  6 files changed, 166 insertions(+), 181 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 7c00656151fb..eda83a37a0c3 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -149,7 +149,7 @@ static int run_deallocate_ex(struct ntfs_sb_info *sbi, struct runs_tree *run,
  int attr_allocate_clusters(struct ntfs_sb_info *sbi, struct runs_tree *run,
  			   CLST vcn, CLST lcn, CLST len, CLST *pre_alloc,
  			   enum ALLOCATE_OPT opt, CLST *alen, const size_t fr,
-			   CLST *new_lcn)
+			   CLST *new_lcn, CLST *new_len)
  {
  	int err;
  	CLST flen, vcn0 = vcn, pre = pre_alloc ? *pre_alloc : 0;
@@ -169,20 +169,36 @@ int attr_allocate_clusters(struct ntfs_sb_info *sbi, struct runs_tree *run,
  		if (err)
  			goto out;
  
-		if (new_lcn && vcn == vcn0)
-			*new_lcn = lcn;
+		if (vcn == vcn0) {
+			/* Return the first fragment. */
+			if (new_lcn)
+				*new_lcn = lcn;
+			if (new_len)
+				*new_len = flen;
+		}
  
  		/* Add new fragment into run storage. */
-		if (!run_add_entry(run, vcn, lcn, flen, opt == ALLOCATE_MFT)) {
+		if (!run_add_entry(run, vcn, lcn, flen, opt & ALLOCATE_MFT)) {
  			/* Undo last 'ntfs_look_for_free_space' */
  			mark_as_free_ex(sbi, lcn, len, false);
  			err = -ENOMEM;
  			goto out;
  		}
  
+		if (opt & ALLOCATE_ZERO) {
+			u8 shift = sbi->cluster_bits - SECTOR_SHIFT;
+
+			err = blkdev_issue_zeroout(sbi->sb->s_bdev,
+						   (sector_t)lcn << shift,
+						   (sector_t)flen << shift,
+						   GFP_NOFS, 0);
+			if (err)
+				goto out;
+		}
+
  		vcn += flen;
  
-		if (flen >= len || opt == ALLOCATE_MFT ||
+		if (flen >= len || (opt & ALLOCATE_MFT) ||
  		    (fr && run->count - cnt >= fr)) {
  			*alen = vcn - vcn0;
  			return 0;
@@ -257,7 +273,8 @@ int attr_make_nonresident(struct ntfs_inode *ni, struct ATTRIB *attr,
  		const char *data = resident_data(attr);
  
  		err = attr_allocate_clusters(sbi, run, 0, 0, len, NULL,
-					     ALLOCATE_DEF, &alen, 0, NULL);
+					     ALLOCATE_DEF, &alen, 0, NULL,
+					     NULL);
  		if (err)
  			goto out1;
  
@@ -552,13 +569,13 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  			/* ~3 bytes per fragment. */
  			err = attr_allocate_clusters(
  				sbi, run, vcn, lcn, to_allocate, &pre_alloc,
-				is_mft ? ALLOCATE_MFT : 0, &alen,
+				is_mft ? ALLOCATE_MFT : ALLOCATE_DEF, &alen,
  				is_mft ? 0
  				       : (sbi->record_size -
  					  le32_to_cpu(rec->used) + 8) /
  							 3 +
  						 1,
-				NULL);
+				NULL, NULL);
  			if (err)
  				goto out;
  		}
@@ -855,8 +872,19 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  	return err;
  }
  
+/*
+ * attr_data_get_block - Returns 'lcn' and 'len' for given 'vcn'.
+ *
+ * @new == NULL means just to get current mapping for 'vcn'
+ * @new != NULL means allocate real cluster if 'vcn' maps to hole
+ * @zero - zeroout new allocated clusters
+ *
+ *  NOTE:
+ *  - @new != NULL is called only for sparsed or compressed attributes.
+ *  - new allocated clusters are zeroed via blkdev_issue_zeroout.
+ */
  int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
-			CLST *len, bool *new)
+			CLST *len, bool *new, bool zero)
  {
  	int err = 0;
  	struct runs_tree *run = &ni->file.run;
@@ -865,29 +893,27 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  	struct ATTRIB *attr = NULL, *attr_b;
  	struct ATTR_LIST_ENTRY *le, *le_b;
  	struct mft_inode *mi, *mi_b;
-	CLST hint, svcn, to_alloc, evcn1, next_svcn, asize, end;
+	CLST hint, svcn, to_alloc, evcn1, next_svcn, asize, end, vcn0, alen;
+	unsigned fr;
  	u64 total_size;
-	u32 clst_per_frame;
-	bool ok;
  
  	if (new)
  		*new = false;
  
+	/* Try to find in cache. */
  	down_read(&ni->file.run_lock);
-	ok = run_lookup_entry(run, vcn, lcn, len, NULL);
+	if (!run_lookup_entry(run, vcn, lcn, len, NULL))
+		*len = 0;
  	up_read(&ni->file.run_lock);
  
-	if (ok && (*lcn != SPARSE_LCN || !new)) {
-		/* Normal way. */
-		return 0;
+	if (*len) {
+		if (*lcn != SPARSE_LCN || !new)
+			return 0; /* Fast normal way without allocation. */
+		else if (clen > *len)
+			clen = *len;
  	}
  
-	if (!clen)
-		clen = 1;
-
-	if (ok && clen > *len)
-		clen = *len;
-
+	/* No cluster in cache or we need to allocate cluster in hole. */
  	sbi = ni->mi.sbi;
  	cluster_bits = sbi->cluster_bits;
  
@@ -913,12 +939,6 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  		goto out;
  	}
  
-	clst_per_frame = 1u << attr_b->nres.c_unit;
-	to_alloc = (clen + clst_per_frame - 1) & ~(clst_per_frame - 1);
-
-	if (vcn + to_alloc > asize)
-		to_alloc = asize - vcn;
-
  	svcn = le64_to_cpu(attr_b->nres.svcn);
  	evcn1 = le64_to_cpu(attr_b->nres.evcn) + 1;
  
@@ -937,36 +957,68 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  		evcn1 = le64_to_cpu(attr->nres.evcn) + 1;
  	}
  
+	/* Load in cache actual information. */
  	err = attr_load_runs(attr, ni, run, NULL);
  	if (err)
  		goto out;
  
-	if (!ok) {
-		ok = run_lookup_entry(run, vcn, lcn, len, NULL);
-		if (ok && (*lcn != SPARSE_LCN || !new)) {
-			/* Normal way. */
-			err = 0;
-			goto ok;
-		}
+	if (!*len) {
+		if (run_lookup_entry(run, vcn, lcn, len, NULL)) {
+			if (*lcn != SPARSE_LCN || !new)
+				goto ok; /* Slow normal way without allocation. */
  
-		if (!ok && !new) {
-			*len = 0;
-			err = 0;
+			if (clen > *len)
+				clen = *len;
+		} else if (!new) {
+			/* Here we may return -ENOENT.
+			 * In any case caller gets zero length. */
  			goto ok;
  		}
-
-		if (ok && clen > *len) {
-			clen = *len;
-			to_alloc = (clen + clst_per_frame - 1) &
-				   ~(clst_per_frame - 1);
-		}
  	}
  
  	if (!is_attr_ext(attr_b)) {
+		/* The code below only for sparsed or compressed attributes. */
  		err = -EINVAL;
  		goto out;
  	}
  
+	vcn0 = vcn;
+	to_alloc = clen;
+	fr = (sbi->record_size - le32_to_cpu(mi->mrec->used) + 8) / 3 + 1;
+	/* Allocate frame aligned clusters.
+	 * ntfs.sys usually uses 16 clusters per frame for sparsed or compressed.
+	 * ntfs3 uses 1 cluster per frame for new created sparsed files. */
+	if (attr_b->nres.c_unit) {
+		CLST clst_per_frame = 1u << attr_b->nres.c_unit;
+		CLST cmask = ~(clst_per_frame - 1);
+
+		/* Get frame aligned vcn and to_alloc. */
+		vcn = vcn0 & cmask;
+		to_alloc = ((vcn0 + clen + clst_per_frame - 1) & cmask) - vcn;
+		if (fr < clst_per_frame)
+			fr = clst_per_frame;
+		zero = true;
+
+		/* Check if 'vcn' and 'vcn0' in different attribute segments. */
+		if (vcn < svcn || evcn1 <= vcn) {
+			/* Load attribute for truncated vcn. */
+			attr = ni_find_attr(ni, attr_b, &le, ATTR_DATA, NULL, 0,
+					    &vcn, &mi);
+			if (!attr) {
+				err = -EINVAL;
+				goto out;
+			}
+			svcn = le64_to_cpu(attr->nres.svcn);
+			evcn1 = le64_to_cpu(attr->nres.evcn) + 1;
+			err = attr_load_runs(attr, ni, run, NULL);
+			if (err)
+				goto out;
+		}
+	}
+
+	if (vcn + to_alloc > asize)
+		to_alloc = asize - vcn;
+
  	/* Get the last LCN to allocate from. */
  	hint = 0;
  
@@ -980,18 +1032,33 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  		hint = -1;
  	}
  
-	err = attr_allocate_clusters(
-		sbi, run, vcn, hint + 1, to_alloc, NULL, 0, len,
-		(sbi->record_size - le32_to_cpu(mi->mrec->used) + 8) / 3 + 1,
-		lcn);
+	/* Allocate and zeroout new clusters. */
+	err = attr_allocate_clusters(sbi, run, vcn, hint + 1, to_alloc, NULL,
+				     zero ? ALLOCATE_ZERO : ALLOCATE_DEF, &alen,
+				     fr, lcn, len);
  	if (err)
  		goto out;
  	*new = true;
  
-	end = vcn + *len;
-
+	end = vcn + alen;
  	total_size = le64_to_cpu(attr_b->nres.total_size) +
-		     ((u64)*len << cluster_bits);
+		     ((u64)alen << cluster_bits);
+
+	if (vcn != vcn0) {
+		if (!run_lookup_entry(run, vcn0, lcn, len, NULL)) {
+			err = -EINVAL;
+			goto out;
+		}
+		if (*lcn == SPARSE_LCN) {
+			/* Internal error. Should not happened. */
+			WARN_ON(1);
+			err = -EINVAL;
+			goto out;
+		}
+		/* Check case when vcn0 + len overlaps new allocated clusters. */
+		if (vcn0 + *len > end)
+			*len = end - vcn0;
+	}
  
  repack:
  	err = mi_pack_runs(mi, attr, run, max(end, evcn1) - svcn);
@@ -1516,7 +1583,7 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size,
  	struct ATTRIB *attr = NULL, *attr_b;
  	struct ATTR_LIST_ENTRY *le, *le_b;
  	struct mft_inode *mi, *mi_b;
-	CLST svcn, evcn1, next_svcn, lcn, len;
+	CLST svcn, evcn1, next_svcn, len;
  	CLST vcn, end, clst_data;
  	u64 total_size, valid_size, data_size;
  
@@ -1592,8 +1659,9 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size,
  		}
  
  		err = attr_allocate_clusters(sbi, run, vcn + clst_data,
-					     hint + 1, len - clst_data, NULL, 0,
-					     &alen, 0, &lcn);
+					     hint + 1, len - clst_data, NULL,
+					     ALLOCATE_DEF, &alen, 0, NULL,
+					     NULL);
  		if (err)
  			goto out;
  
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 96ba3f5a8470..63aef132e529 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -122,8 +122,8 @@ static int ntfs_extend_initialized_size(struct file *file,
  			bits = sbi->cluster_bits;
  			vcn = pos >> bits;
  
-			err = attr_data_get_block(ni, vcn, 0, &lcn, &clen,
-						  NULL);
+			err = attr_data_get_block(ni, vcn, 1, &lcn, &clen, NULL,
+						  false);
  			if (err)
  				goto out;
  
@@ -180,18 +180,18 @@ static int ntfs_zero_range(struct inode *inode, u64 vbo, u64 vbo_to)
  	struct address_space *mapping = inode->i_mapping;
  	u32 blocksize = 1 << inode->i_blkbits;
  	pgoff_t idx = vbo >> PAGE_SHIFT;
-	u32 z_start = vbo & (PAGE_SIZE - 1);
+	u32 from = vbo & (PAGE_SIZE - 1);
  	pgoff_t idx_end = (vbo_to + PAGE_SIZE - 1) >> PAGE_SHIFT;
  	loff_t page_off;
  	struct buffer_head *head, *bh;
-	u32 bh_next, bh_off, z_end;
+	u32 bh_next, bh_off, to;
  	sector_t iblock;
  	struct page *page;
  
-	for (; idx < idx_end; idx += 1, z_start = 0) {
+	for (; idx < idx_end; idx += 1, from = 0) {
  		page_off = (loff_t)idx << PAGE_SHIFT;
-		z_end = (page_off + PAGE_SIZE) > vbo_to ? (vbo_to - page_off)
-							: PAGE_SIZE;
+		to = (page_off + PAGE_SIZE) > vbo_to ? (vbo_to - page_off)
+						     : PAGE_SIZE;
  		iblock = page_off >> inode->i_blkbits;
  
  		page = find_or_create_page(mapping, idx,
@@ -208,7 +208,7 @@ static int ntfs_zero_range(struct inode *inode, u64 vbo, u64 vbo_to)
  		do {
  			bh_next = bh_off + blocksize;
  
-			if (bh_next <= z_start || bh_off >= z_end)
+			if (bh_next <= from || bh_off >= to)
  				continue;
  
  			if (!buffer_mapped(bh)) {
@@ -242,7 +242,7 @@ static int ntfs_zero_range(struct inode *inode, u64 vbo, u64 vbo_to)
  		} while (bh_off = bh_next, iblock += 1,
  			 head != (bh = bh->b_this_page));
  
-		zero_user_segment(page, z_start, z_end);
+		zero_user_segment(page, from, to);
  
  		unlock_page(page);
  		put_page(page);
@@ -253,80 +253,6 @@ static int ntfs_zero_range(struct inode *inode, u64 vbo, u64 vbo_to)
  	return err;
  }
  
-/*
- * ntfs_sparse_cluster - Helper function to zero a new allocated clusters.
- *
- * NOTE: 512 <= cluster size <= 2M
- */
-void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
-			 CLST len)
-{
-	struct address_space *mapping = inode->i_mapping;
-	struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info;
-	u8 cluster_bits = sbi->cluster_bits;
-	u64 vbo = (u64)vcn << cluster_bits;
-	u64 bytes = (u64)len << cluster_bits;
-	u32 blocksize = 1 << inode->i_blkbits;
-	pgoff_t idx0 = page0 ? page0->index : -1;
-	loff_t vbo_clst = vbo & sbi->cluster_mask_inv;
-	loff_t end = ntfs_up_cluster(sbi, vbo + bytes);
-	pgoff_t idx = vbo_clst >> PAGE_SHIFT;
-	u32 from = vbo_clst & (PAGE_SIZE - 1);
-	pgoff_t idx_end = (end + PAGE_SIZE - 1) >> PAGE_SHIFT;
-	loff_t page_off;
-	u32 to;
-	bool partial;
-	struct page *page;
-
-	for (; idx < idx_end; idx += 1, from = 0) {
-		page = idx == idx0 ? page0 : grab_cache_page(mapping, idx);
-
-		if (!page)
-			continue;
-
-		page_off = (loff_t)idx << PAGE_SHIFT;
-		to = (page_off + PAGE_SIZE) > end ? (end - page_off)
-						  : PAGE_SIZE;
-		partial = false;
-
-		if ((from || PAGE_SIZE != to) &&
-		    likely(!page_has_buffers(page))) {
-			create_empty_buffers(page, blocksize, 0);
-		}
-
-		if (page_has_buffers(page)) {
-			struct buffer_head *head, *bh;
-			u32 bh_off = 0;
-
-			bh = head = page_buffers(page);
-			do {
-				u32 bh_next = bh_off + blocksize;
-
-				if (from <= bh_off && bh_next <= to) {
-					set_buffer_uptodate(bh);
-					mark_buffer_dirty(bh);
-				} else if (!buffer_uptodate(bh)) {
-					partial = true;
-				}
-				bh_off = bh_next;
-			} while (head != (bh = bh->b_this_page));
-		}
-
-		zero_user_segment(page, from, to);
-
-		if (!partial)
-			SetPageUptodate(page);
-		flush_dcache_page(page);
-		set_page_dirty(page);
-
-		if (idx != idx0) {
-			unlock_page(page);
-			put_page(page);
-		}
-		cond_resched();
-	}
-}
-
  /*
   * ntfs_file_mmap - file_operations::mmap
   */
@@ -368,13 +294,9 @@ static int ntfs_file_mmap(struct file *file, struct vm_area_struct *vma)
  
  			for (; vcn < end; vcn += len) {
  				err = attr_data_get_block(ni, vcn, 1, &lcn,
-							  &len, &new);
+							  &len, &new, true);
  				if (err)
  					goto out;
-
-				if (!new)
-					continue;
-				ntfs_sparse_cluster(inode, NULL, vcn, 1);
  			}
  		}
  
@@ -518,7 +440,8 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  	struct ntfs_sb_info *sbi = sb->s_fs_info;
  	struct ntfs_inode *ni = ntfs_i(inode);
  	loff_t end = vbo + len;
-	loff_t vbo_down = round_down(vbo, PAGE_SIZE);
+	loff_t vbo_down = round_down(vbo, max_t(unsigned long,
+						sbi->cluster_size, PAGE_SIZE));
  	bool is_supported_holes = is_sparsed(ni) || is_compressed(ni);
  	loff_t i_size, new_size;
  	bool map_locked;
@@ -571,7 +494,8 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  		u32 frame_size;
  		loff_t mask, vbo_a, end_a, tmp;
  
-		err = filemap_write_and_wait_range(mapping, vbo, LLONG_MAX);
+		err = filemap_write_and_wait_range(mapping, vbo_down,
+						   LLONG_MAX);
  		if (err)
  			goto out;
  
@@ -672,39 +596,35 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  			goto out;
  
  		if (is_supported_holes) {
-			CLST vcn_v = bytes_to_cluster(sbi, ni->i_valid);
  			CLST vcn = vbo >> sbi->cluster_bits;
  			CLST cend = bytes_to_cluster(sbi, end);
+			CLST cend_v = bytes_to_cluster(sbi, ni->i_valid);
  			CLST lcn, clen;
  			bool new;
  
+			if (cend_v > cend)
+				cend_v = cend;
+
  			/*
-			 * Allocate but do not zero new clusters. (see below comments)
-			 * This breaks security: One can read unused on-disk areas.
+			 * Allocate and zero new clusters.
  			 * Zeroing these clusters may be too long.
-			 * Maybe we should check here for root rights?
+			 */
+			for (; vcn < cend_v; vcn += clen) {
+				err = attr_data_get_block(ni, vcn, cend_v - vcn,
+							  &lcn, &clen, &new,
+							  true);
+				if (err)
+					goto out;
+			}
+			/*
+			 * Allocate but not zero new clusters.
  			 */
  			for (; vcn < cend; vcn += clen) {
  				err = attr_data_get_block(ni, vcn, cend - vcn,
-							  &lcn, &clen, &new);
+							  &lcn, &clen, &new,
+							  false);
  				if (err)
  					goto out;
-				if (!new || vcn >= vcn_v)
-					continue;
-
-				/*
-				 * Unwritten area.
-				 * NTFS is not able to store several unwritten areas.
-				 * Activate 'ntfs_sparse_cluster' to zero new allocated clusters.
-				 *
-				 * Dangerous in case:
-				 * 1G of sparsed clusters + 1 cluster of data =>
-				 * valid_size == 1G + 1 cluster
-				 * fallocate(1G) will zero 1G and this can be very long
-				 * xfstest 016/086 will fail without 'ntfs_sparse_cluster'.
-				 */
-				ntfs_sparse_cluster(inode, NULL, vcn,
-						    min(vcn_v - vcn, clen));
  			}
  		}
  
@@ -925,8 +845,8 @@ static ssize_t ntfs_compress_write(struct kiocb *iocb, struct iov_iter *from)
  		frame_vbo = valid & ~(frame_size - 1);
  		off = valid & (frame_size - 1);
  
-		err = attr_data_get_block(ni, frame << NTFS_LZNT_CUNIT, 0, &lcn,
-					  &clen, NULL);
+		err = attr_data_get_block(ni, frame << NTFS_LZNT_CUNIT, 1, &lcn,
+					  &clen, NULL, false);
  		if (err)
  			goto out;
  
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index a7aed31e7c93..370c6398a044 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -2224,7 +2224,7 @@ int ni_decompress_file(struct ntfs_inode *ni)
  
  		for (vcn = vbo >> sbi->cluster_bits; vcn < end; vcn += clen) {
  			err = attr_data_get_block(ni, vcn, cend - vcn, &lcn,
-						  &clen, &new);
+						  &clen, &new, false);
  			if (err)
  				goto out;
  		}
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 50c90d7e8a78..bc9ab93db1d0 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1347,8 +1347,8 @@ static int indx_create_allocate(struct ntfs_index *indx, struct ntfs_inode *ni,
  
  	run_init(&run);
  
-	err = attr_allocate_clusters(sbi, &run, 0, 0, len, NULL, 0, &alen, 0,
-				     NULL);
+	err = attr_allocate_clusters(sbi, &run, 0, 0, len, NULL, ALLOCATE_DEF,
+				     &alen, 0, NULL, NULL);
  	if (err)
  		goto out;
  
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index f487d36c9b78..18edbc7b35df 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -576,7 +576,8 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
  	off = vbo & sbi->cluster_mask;
  	new = false;
  
-	err = attr_data_get_block(ni, vcn, 1, &lcn, &len, create ? &new : NULL);
+	err = attr_data_get_block(ni, vcn, 1, &lcn, &len, create ? &new : NULL,
+				  create && sbi->cluster_size > PAGE_SIZE);
  	if (err)
  		goto out;
  
@@ -594,11 +595,8 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
  		WARN_ON(1);
  	}
  
-	if (new) {
+	if (new)
  		set_buffer_new(bh);
-		if ((len << cluster_bits) > block_size)
-			ntfs_sparse_cluster(inode, page, vcn, len);
-	}
  
  	lbo = ((u64)lcn << cluster_bits) + off;
  
@@ -1529,8 +1527,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  				cpu_to_le64(ntfs_up_cluster(sbi, nsize));
  
  			err = attr_allocate_clusters(sbi, &ni->file.run, 0, 0,
-						     clst, NULL, 0, &alen, 0,
-						     NULL);
+						     clst, NULL, ALLOCATE_DEF,
+						     &alen, 0, NULL, NULL);
  			if (err)
  				goto out5;
  
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index e9f6898ec924..c45a411f82f6 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -126,6 +126,7 @@ struct ntfs_buffers {
  enum ALLOCATE_OPT {
  	ALLOCATE_DEF = 0, // Allocate all clusters.
  	ALLOCATE_MFT = 1, // Allocate for MFT.
+	ALLOCATE_ZERO = 2, // Zeroout new allocated clusters
  };
  
  enum bitmap_mutex_classes {
@@ -414,7 +415,7 @@ enum REPARSE_SIGN {
  int attr_allocate_clusters(struct ntfs_sb_info *sbi, struct runs_tree *run,
  			   CLST vcn, CLST lcn, CLST len, CLST *pre_alloc,
  			   enum ALLOCATE_OPT opt, CLST *alen, const size_t fr,
-			   CLST *new_lcn);
+			   CLST *new_lcn, CLST *new_len);
  int attr_make_nonresident(struct ntfs_inode *ni, struct ATTRIB *attr,
  			  struct ATTR_LIST_ENTRY *le, struct mft_inode *mi,
  			  u64 new_size, struct runs_tree *run,
@@ -424,7 +425,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  		  u64 new_size, const u64 *new_valid, bool keep_prealloc,
  		  struct ATTRIB **ret);
  int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
-			CLST *len, bool *new);
+			CLST *len, bool *new, bool zero);
  int attr_data_read_resident(struct ntfs_inode *ni, struct page *page);
  int attr_data_write_resident(struct ntfs_inode *ni, struct page *page);
  int attr_load_runs_vcn(struct ntfs_inode *ni, enum ATTR_TYPE type,
@@ -489,8 +490,6 @@ extern const struct file_operations ntfs_dir_operations;
  /* Globals from file.c */
  int ntfs_getattr(struct user_namespace *mnt_userns, const struct path *path,
  		 struct kstat *stat, u32 request_mask, u32 flags);
-void ntfs_sparse_cluster(struct inode *inode, struct page *page0, CLST vcn,
-			 CLST len);
  int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
  		  struct iattr *attr);
  int ntfs_file_open(struct inode *inode, struct file *file);
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/14] fs/ntfs3: Fix wrong indentations
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
  2022-10-28 17:01 ` [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters Konstantin Komarov
  2022-10-28 17:02 ` [PATCH 02/14] fs/ntfs3: Change new sparse cluster processing Konstantin Komarov
@ 2022-10-28 17:02 ` Konstantin Komarov
  2022-10-28 17:03 ` [PATCH 04/14] fs/ntfs3: atomic_open implementation Konstantin Komarov
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:02 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Also simplifying code.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/fslog.c | 3 +--
  fs/ntfs3/index.c | 8 ++++----
  fs/ntfs3/inode.c | 8 +++++---
  3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 5289c25b1ee4..e61545b9772e 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -4824,8 +4824,7 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
  		goto out;
  	}
  	attr = oa->attr;
-	t64 = le64_to_cpu(attr->nres.alloc_size);
-	if (size > t64) {
+	if (size > le64_to_cpu(attr->nres.alloc_size)) {
  		attr->nres.valid_size = attr->nres.data_size =
  			attr->nres.alloc_size = cpu_to_le64(size);
  	}
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index bc9ab93db1d0..a2e1e07b5bb8 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -625,9 +625,8 @@ void fnd_clear(struct ntfs_fnd *fnd)
  static int fnd_push(struct ntfs_fnd *fnd, struct indx_node *n,
  		    struct NTFS_DE *e)
  {
-	int i;
+	int i = fnd->level;
  
-	i = fnd->level;
  	if (i < 0 || i >= ARRAY_SIZE(fnd->nodes))
  		return -EINVAL;
  	fnd->nodes[i] = n;
@@ -2121,9 +2120,10 @@ static int indx_get_entry_to_replace(struct ntfs_index *indx,
  	fnd->de[level] = e;
  	indx_write(indx, ni, n, 0);
  
-	/* Check to see if this action created an empty leaf. */
-	if (ib_is_leaf(ib) && ib_is_empty(ib))
+	if (ib_is_leaf(ib) && ib_is_empty(ib)) {
+		/* An empty leaf. */
  		return 0;
+	}
  
  out:
  	fnd_clear(fnd);
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 18edbc7b35df..df0d30a3218a 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1810,11 +1810,12 @@ static int ntfs_translate_junction(const struct super_block *sb,
  
  	/* Make translated path a relative path to mount point */
  	strcpy(translated, "./");
-	++link_path;	/* Skip leading / */
+	++link_path; /* Skip leading / */
  	for (tl_len = sizeof("./") - 1; *link_path; ++link_path) {
  		if (*link_path == '/') {
  			if (PATH_MAX - tl_len < sizeof("../")) {
-				ntfs_err(sb, "Link path %s has too many components",
+				ntfs_err(sb,
+					 "Link path %s has too many components",
  					 link_path);
  				err = -EINVAL;
  				goto out;
@@ -1830,7 +1831,8 @@ static int ntfs_translate_junction(const struct super_block *sb,
  		++target_start;
  
  	if (!*target_start) {
-		ntfs_err(sb, "Link target (%s) missing drive separator", target);
+		ntfs_err(sb, "Link target (%s) missing drive separator",
+			 target);
  		err = -EINVAL;
  		goto out;
  	}
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/14] fs/ntfs3: atomic_open implementation
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (2 preceding siblings ...)
  2022-10-28 17:02 ` [PATCH 03/14] fs/ntfs3: Fix wrong indentations Konstantin Komarov
@ 2022-10-28 17:03 ` Konstantin Komarov
  2022-10-28 17:03 ` [PATCH 05/14] fs/ntfs3: Fixing wrong logic in attr_set_size and ntfs_fallocate Konstantin Komarov
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:03 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Added ntfs_atomic_open function.
Relaxed locking in ntfs_create_inode.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/inode.c |  24 ++++++++++--
  fs/ntfs3/namei.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index df0d30a3218a..405afb54cc19 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1183,6 +1183,18 @@ ntfs_create_reparse_buffer(struct ntfs_sb_info *sbi, const char *symname,
  	return ERR_PTR(err);
  }
  
+/*
+ * ntfs_create_inode
+ *
+ * Helper function for:
+ * - ntfs_create
+ * - ntfs_mknod
+ * - ntfs_symlink
+ * - ntfs_mkdir
+ * - ntfs_atomic_open
+ *
+ * NOTE: if fnd != NULL (ntfs_atomic_open) then @dir is locked
+ */
  struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  				struct inode *dir, struct dentry *dentry,
  				const struct cpu_str *uni, umode_t mode,
@@ -1212,7 +1224,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  	struct REPARSE_DATA_BUFFER *rp = NULL;
  	bool rp_inserted = false;
  
-	ni_lock_dir(dir_ni);
+	if (!fnd)
+		ni_lock_dir(dir_ni);
  
  	dir_root = indx_get_root(&dir_ni->dir, dir_ni, NULL, NULL);
  	if (!dir_root) {
@@ -1575,7 +1588,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  		goto out6;
  
  	/* Unlock parent directory before ntfs_init_acl. */
-	ni_unlock(dir_ni);
+	if (!fnd)
+		ni_unlock(dir_ni);
  
  	inode->i_generation = le16_to_cpu(rec->seq);
  
@@ -1635,7 +1649,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  out7:
  
  	/* Undo 'indx_insert_entry'. */
-	ni_lock_dir(dir_ni);
+	if (!fnd)
+		ni_lock_dir(dir_ni);
  	indx_delete_entry(&dir_ni->dir, dir_ni, new_de + 1,
  			  le16_to_cpu(new_de->key_size), sbi);
  	/* ni_unlock(dir_ni); will be called later. */
@@ -1663,7 +1678,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  
  out1:
  	if (err) {
-		ni_unlock(dir_ni);
+		if (!fnd)
+			ni_unlock(dir_ni);
  		return ERR_PTR(err);
  	}
  
diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index ff76389475ad..1af02d4f6b4d 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -8,6 +8,7 @@
  #include <linux/fs.h>
  #include <linux/nls.h>
  #include <linux/ctype.h>
+#include <linux/posix_acl.h>
  
  #include "debug.h"
  #include "ntfs.h"
@@ -334,6 +335,104 @@ static int ntfs_rename(struct user_namespace *mnt_userns, struct inode *dir,
  	return err;
  }
  
+/*
+ * ntfs_atomic_open
+ *
+ * inode_operations::atomic_open
+ */
+static int ntfs_atomic_open(struct inode *dir, struct dentry *dentry,
+			    struct file *file, u32 flags, umode_t mode)
+{
+	int err;
+	struct inode *inode;
+	struct ntfs_fnd *fnd = NULL;
+	struct ntfs_inode *ni = ntfs_i(dir);
+	struct dentry *d = NULL;
+	struct cpu_str *uni = __getname();
+	bool locked = false;
+
+	if (!uni)
+		return -ENOMEM;
+
+	err = ntfs_nls_to_utf16(ni->mi.sbi, dentry->d_name.name,
+				dentry->d_name.len, uni, NTFS_NAME_LEN,
+				UTF16_HOST_ENDIAN);
+	if (err < 0)
+		goto out;
+
+#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
+		 */
+		struct posix_acl *p = get_acl(dir, ACL_TYPE_DEFAULT);
+
+		if (IS_ERR(p)) {
+			err = PTR_ERR(p);
+			goto out;
+		}
+		posix_acl_release(p);
+	}
+#endif
+
+	if (d_in_lookup(dentry)) {
+		ni_lock_dir(ni);
+		locked = true;
+		fnd = fnd_get();
+		if (!fnd) {
+			err = -ENOMEM;
+			goto out1;
+		}
+
+		d = d_splice_alias(dir_search_u(dir, uni, fnd), dentry);
+		if (IS_ERR(d)) {
+			err = PTR_ERR(d);
+			d = NULL;
+			goto out2;
+		}
+
+		if (d)
+			dentry = d;
+	}
+
+	if (!(flags & O_CREAT) || d_really_is_positive(dentry)) {
+		err = finish_no_open(file, d);
+		goto out2;
+	}
+
+	file->f_mode |= FMODE_CREATED;
+
+	/*
+	 * fnd contains tree's path to insert to.
+	 * If fnd is not NULL then dir is locked.
+	 */
+
+	/*
+	 * Unfortunately I don't know how to get here correct 'struct nameidata *nd'
+	 * or 'struct user_namespace *mnt_userns'.
+	 * See atomic_open in fs/namei.c.
+	 * This is why xfstest/633 failed.
+	 * Looks like ntfs_atomic_open must accept 'struct user_namespace *mnt_userns' as argument.
+	 */
+
+	inode = ntfs_create_inode(&init_user_ns, dir, dentry, uni, mode, 0,
+				  NULL, 0, fnd);
+	err = IS_ERR(inode) ? PTR_ERR(inode)
+			    : finish_open(file, dentry, ntfs_file_open);
+	dput(d);
+
+out2:
+	fnd_put(fnd);
+out1:
+	if (locked)
+		ni_unlock(ni);
+out:
+	__putname(uni);
+	return err;
+}
+
  struct dentry *ntfs3_get_parent(struct dentry *child)
  {
  	struct inode *inode = d_inode(child);
@@ -504,6 +603,7 @@ const struct inode_operations ntfs_dir_inode_operations = {
  	.setattr	= ntfs3_setattr,
  	.getattr	= ntfs_getattr,
  	.listxattr	= ntfs_listxattr,
+	.atomic_open	= ntfs_atomic_open,
  	.fiemap		= ntfs_fiemap,
  };
  
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/14] fs/ntfs3: Fixing wrong logic in attr_set_size and ntfs_fallocate
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (3 preceding siblings ...)
  2022-10-28 17:03 ` [PATCH 04/14] fs/ntfs3: atomic_open implementation Konstantin Komarov
@ 2022-10-28 17:03 ` Konstantin Komarov
  2022-10-28 17:04 ` [PATCH 06/14] fs/ntfs3: Changing locking in ntfs_rename Konstantin Komarov
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:03 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

There were 2 problems:
- in some cases we lost dirty flag;
- cluster allocation can be called even when it wasn't needed.
Fixes xfstest generic/465

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c | 25 +++++++++++--------------
  fs/ntfs3/file.c   | 30 ++++++++++++++++++------------
  fs/ntfs3/index.c  |  9 +++++++++
  fs/ntfs3/inode.c  | 17 +++++------------
  4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index eda83a37a0c3..91ea73e6f4fe 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -414,6 +414,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  	CLST alen, vcn, lcn, new_alen, old_alen, svcn, evcn;
  	CLST next_svcn, pre_alloc = -1, done = 0;
  	bool is_ext, is_bad = false;
+	bool dirty = false;
  	u32 align;
  	struct MFT_REC *rec;
  
@@ -434,8 +435,10 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  			return err;
  
  		/* Return if file is still resident. */
-		if (!attr_b->non_res)
+		if (!attr_b->non_res) {
+			dirty = true;
  			goto ok1;
+		}
  
  		/* Layout of records may be changed, so do a full search. */
  		goto again;
@@ -458,7 +461,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  
  	if (keep_prealloc && new_size < old_size) {
  		attr_b->nres.data_size = cpu_to_le64(new_size);
-		mi_b->dirty = true;
+		mi_b->dirty = dirty = true;
  		goto ok;
  	}
  
@@ -504,7 +507,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  
  		if (new_alloc <= old_alloc) {
  			attr_b->nres.data_size = cpu_to_le64(new_size);
-			mi_b->dirty = true;
+			mi_b->dirty = dirty = true;
  			goto ok;
  		}
  
@@ -595,7 +598,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  		next_svcn = le64_to_cpu(attr->nres.evcn) + 1;
  		new_alloc_tmp = (u64)next_svcn << cluster_bits;
  		attr_b->nres.alloc_size = cpu_to_le64(new_alloc_tmp);
-		mi_b->dirty = true;
+		mi_b->dirty = dirty = true;
  
  		if (next_svcn >= vcn && !to_allocate) {
  			/* Normal way. Update attribute and exit. */
@@ -681,7 +684,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  		old_valid = old_size = old_alloc = (u64)vcn << cluster_bits;
  		attr_b->nres.valid_size = attr_b->nres.data_size =
  			attr_b->nres.alloc_size = cpu_to_le64(old_size);
-		mi_b->dirty = true;
+		mi_b->dirty = dirty = true;
  		goto again_1;
  	}
  
@@ -743,7 +746,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  				attr_b->nres.valid_size =
  					attr_b->nres.alloc_size;
  		}
-		mi_b->dirty = true;
+		mi_b->dirty = dirty = true;
  
  		err = run_deallocate_ex(sbi, run, vcn, evcn - vcn + 1, &dlen,
  					true);
@@ -804,16 +807,9 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  	if (ret)
  		*ret = attr_b;
  
-	/* Update inode_set_bytes. */
  	if (((type == ATTR_DATA && !name_len) ||
  	     (type == ATTR_ALLOC && name == I30_NAME))) {
-		bool dirty = false;
-
-		if (ni->vfs_inode.i_size != new_size) {
-			ni->vfs_inode.i_size = new_size;
-			dirty = true;
-		}
-
+		/* Update inode_set_bytes. */
  		if (attr_b->non_res) {
  			new_alloc = le64_to_cpu(attr_b->nres.alloc_size);
  			if (inode_get_bytes(&ni->vfs_inode) != new_alloc) {
@@ -822,6 +818,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
  			}
  		}
  
+		/* Don't forget to update duplicate information in parent. */
  		if (dirty) {
  			ni->ni_flags |= NI_FLAG_UPDATE_PARENT;
  			mark_inode_dirty(&ni->vfs_inode);
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 63aef132e529..511e58f2b0f8 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -337,7 +337,6 @@ static int ntfs_extend(struct inode *inode, loff_t pos, size_t count,
  		err = ntfs_set_size(inode, end);
  		if (err)
  			goto out;
-		inode->i_size = end;
  	}
  
  	if (extend_init && !is_compressed(ni)) {
@@ -588,12 +587,14 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  		if (err)
  			goto out;
  
-		/*
-		 * Allocate clusters, do not change 'valid' size.
-		 */
-		err = ntfs_set_size(inode, new_size);
-		if (err)
-			goto out;
+		if (new_size > i_size) {
+			/*
+			 * Allocate clusters, do not change 'valid' size.
+			 */
+			err = ntfs_set_size(inode, new_size);
+			if (err)
+				goto out;
+		}
  
  		if (is_supported_holes) {
  			CLST vcn = vbo >> sbi->cluster_bits;
@@ -635,6 +636,8 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
  					    &ni->file.run, i_size, &ni->i_valid,
  					    true, NULL);
  			ni_unlock(ni);
+		} else if (new_size > i_size) {
+			inode->i_size = new_size;
  		}
  	}
  
@@ -678,7 +681,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
  		goto out;
  
  	if (ia_valid & ATTR_SIZE) {
-		loff_t oldsize = inode->i_size;
+		loff_t newsize, oldsize;
  
  		if (WARN_ON(ni->ni_flags & NI_FLAG_COMPRESSED_MASK)) {
  			/* Should never be here, see ntfs_file_open(). */
@@ -686,16 +689,19 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
  			goto out;
  		}
  		inode_dio_wait(inode);
+		oldsize = inode->i_size;
+		newsize = attr->ia_size;
  
-		if (attr->ia_size <= oldsize)
-			err = ntfs_truncate(inode, attr->ia_size);
-		else if (attr->ia_size > oldsize)
-			err = ntfs_extend(inode, attr->ia_size, 0, NULL);
+		if (newsize <= oldsize)
+			err = ntfs_truncate(inode, newsize);
+		else
+			err = ntfs_extend(inode, newsize, 0, NULL);
  
  		if (err)
  			goto out;
  
  		ni->ni_flags |= NI_FLAG_UPDATE_PARENT;
+		inode->i_size = newsize;
  	}
  
  	setattr_copy(mnt_userns, inode, attr);
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index a2e1e07b5bb8..35369ae5c438 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1445,6 +1445,9 @@ static int indx_add_allocate(struct ntfs_index *indx, struct ntfs_inode *ni,
  		goto out1;
  	}
  
+	if (in->name == I30_NAME)
+		ni->vfs_inode.i_size = data_size;
+
  	*vbn = bit << indx->idx2vbn_bits;
  
  	return 0;
@@ -1978,6 +1981,9 @@ static int indx_shrink(struct ntfs_index *indx, struct ntfs_inode *ni,
  	if (err)
  		return err;
  
+	if (in->name == I30_NAME)
+		ni->vfs_inode.i_size = new_data;
+
  	bpb = bitmap_size(bit);
  	if (bpb * 8 == nbits)
  		return 0;
@@ -2461,6 +2467,9 @@ int indx_delete_entry(struct ntfs_index *indx, struct ntfs_inode *ni,
  
  		err = attr_set_size(ni, ATTR_ALLOC, in->name, in->name_len,
  				    &indx->alloc_run, 0, NULL, false, NULL);
+		if (in->name == I30_NAME)
+			ni->vfs_inode.i_size = 0;
+
  		err = ni_remove_attr(ni, ATTR_ALLOC, in->name, in->name_len,
  				     false, NULL);
  		run_close(&indx->alloc_run);
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 405afb54cc19..78ec3e6bbf67 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -550,17 +550,6 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
  	clear_buffer_new(bh);
  	clear_buffer_uptodate(bh);
  
-	/* Direct write uses 'create=0'. */
-	if (!create && vbo >= ni->i_valid) {
-		/* Out of valid. */
-		return 0;
-	}
-
-	if (vbo >= inode->i_size) {
-		/* Out of size. */
-		return 0;
-	}
-
  	if (is_resident(ni)) {
  		ni_lock(ni);
  		err = attr_data_read_resident(ni, page);
@@ -624,7 +613,6 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
  		}
  	} else if (vbo >= valid) {
  		/* Read out of valid data. */
-		/* Should never be here 'cause already checked. */
  		clear_buffer_mapped(bh);
  	} else if (vbo + bytes <= valid) {
  		/* Normal read. */
@@ -974,6 +962,11 @@ int ntfs_write_end(struct file *file, struct address_space *mapping,
  			dirty = true;
  		}
  
+		if (pos + err > inode->i_size) {
+			inode->i_size = pos + err;
+			dirty = true;
+		}
+
  		if (dirty)
  			mark_inode_dirty(inode);
  	}
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/14] fs/ntfs3: Changing locking in ntfs_rename
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (4 preceding siblings ...)
  2022-10-28 17:03 ` [PATCH 05/14] fs/ntfs3: Fixing wrong logic in attr_set_size and ntfs_fallocate Konstantin Komarov
@ 2022-10-28 17:04 ` Konstantin Komarov
  2022-10-28 17:04 ` [PATCH 07/14] fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block Konstantin Komarov
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:04 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

In some cases we can be in deadlock
because we tried to lock the same dir.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/namei.c   | 4 ++++
  fs/ntfs3/ntfs_fs.h | 6 ++++++
  2 files changed, 10 insertions(+)

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index 1af02d4f6b4d..13d6acc0747f 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -305,6 +305,8 @@ static int ntfs_rename(struct user_namespace *mnt_userns, struct inode *dir,
  
  	ni_lock_dir(dir_ni);
  	ni_lock(ni);
+	if (dir_ni != new_dir_ni)
+		ni_lock_dir2(new_dir_ni);
  
  	is_bad = false;
  	err = ni_rename(dir_ni, new_dir_ni, ni, de, new_de, &is_bad);
@@ -328,6 +330,8 @@ static int ntfs_rename(struct user_namespace *mnt_userns, struct inode *dir,
  			ntfs_sync_inode(inode);
  	}
  
+	if (dir_ni != new_dir_ni)
+		ni_unlock(new_dir_ni);
  	ni_unlock(ni);
  	ni_unlock(dir_ni);
  out:
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index c45a411f82f6..5fad93a2c3fd 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -333,6 +333,7 @@ enum ntfs_inode_mutex_lock_class {
  	NTFS_INODE_MUTEX_REPARSE,
  	NTFS_INODE_MUTEX_NORMAL,
  	NTFS_INODE_MUTEX_PARENT,
+	NTFS_INODE_MUTEX_PARENT2,
  };
  
  /*
@@ -1119,6 +1120,11 @@ static inline void ni_lock_dir(struct ntfs_inode *ni)
  	mutex_lock_nested(&ni->ni_lock, NTFS_INODE_MUTEX_PARENT);
  }
  
+static inline void ni_lock_dir2(struct ntfs_inode *ni)
+{
+	mutex_lock_nested(&ni->ni_lock, NTFS_INODE_MUTEX_PARENT2);
+}
+
  static inline void ni_unlock(struct ntfs_inode *ni)
  {
  	mutex_unlock(&ni->ni_lock);
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/14] fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (5 preceding siblings ...)
  2022-10-28 17:04 ` [PATCH 06/14] fs/ntfs3: Changing locking in ntfs_rename Konstantin Komarov
@ 2022-10-28 17:04 ` Konstantin Komarov
  2022-10-28 17:05 ` [PATCH 08/14] fs/ntfs3: Correct ntfs_check_for_free_space Konstantin Komarov
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:04 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Added new function ntfs_check_for_free_space.
Added undo mechanism in attr_data_get_block.
Fixes xfstest generic/083

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/attrib.c  | 141 +++++++++++++++++++++++++++++----------------
  fs/ntfs3/fsntfs.c  |  33 +++++++++++
  fs/ntfs3/ntfs_fs.h |   1 +
  3 files changed, 125 insertions(+), 50 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 91ea73e6f4fe..5e6bafb10f42 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -891,8 +891,10 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  	struct ATTR_LIST_ENTRY *le, *le_b;
  	struct mft_inode *mi, *mi_b;
  	CLST hint, svcn, to_alloc, evcn1, next_svcn, asize, end, vcn0, alen;
+	CLST alloc, evcn;
  	unsigned fr;
-	u64 total_size;
+	u64 total_size, total_size0;
+	int step = 0;
  
  	if (new)
  		*new = false;
@@ -932,7 +934,12 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  
  	asize = le64_to_cpu(attr_b->nres.alloc_size) >> cluster_bits;
  	if (vcn >= asize) {
-		err = -EINVAL;
+		if (new) {
+			err = -EINVAL;
+		} else {
+			*len = 1;
+			*lcn = SPARSE_LCN;
+		}
  		goto out;
  	}
  
@@ -1036,10 +1043,12 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  	if (err)
  		goto out;
  	*new = true;
+	step = 1;
  
  	end = vcn + alen;
-	total_size = le64_to_cpu(attr_b->nres.total_size) +
-		     ((u64)alen << cluster_bits);
+	/* Save 'total_size0' to restore if error. */
+	total_size0 = le64_to_cpu(attr_b->nres.total_size);
+	total_size = total_size0 + ((u64)alen << cluster_bits);
  
  	if (vcn != vcn0) {
  		if (!run_lookup_entry(run, vcn0, lcn, len, NULL)) {
@@ -1081,7 +1090,7 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  		if (!ni->attr_list.size) {
  			err = ni_create_attr_list(ni);
  			if (err)
-				goto out;
+				goto undo1;
  			/* Layout of records is changed. */
  			le_b = NULL;
  			attr_b = ni_find_attr(ni, NULL, &le_b, ATTR_DATA, NULL,
@@ -1098,67 +1107,83 @@ 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
+	 * in 'ni_insert_nonresident'.
+	 * Return in advance -ENOSPC here if there are no free cluster and no free MFT.
+	 */
+	if (!ntfs_check_for_free_space(sbi, 1, 1)) {
+		/* Undo step 1. */
+		err = -ENOSPC;
+		goto undo1;
+	}
+
+	step = 2;
  	svcn = evcn1;
  
  	/* Estimate next attribute. */
  	attr = ni_find_attr(ni, attr, &le, ATTR_DATA, NULL, 0, &svcn, &mi);
  
-	if (attr) {
-		CLST alloc = bytes_to_cluster(
-			sbi, le64_to_cpu(attr_b->nres.alloc_size));
-		CLST evcn = le64_to_cpu(attr->nres.evcn);
-
-		if (end < next_svcn)
-			end = next_svcn;
-		while (end > evcn) {
-			/* Remove segment [svcn : evcn). */
-			mi_remove_attr(NULL, mi, attr);
-
-			if (!al_remove_le(ni, le)) {
-				err = -EINVAL;
-				goto out;
-			}
+	if (!attr) {
+		/* Insert new attribute segment. */
+		goto ins_ext;
+	}
  
-			if (evcn + 1 >= alloc) {
-				/* Last attribute segment. */
-				evcn1 = evcn + 1;
-				goto ins_ext;
-			}
+	/* Try to update existed attribute segment. */
+	alloc = bytes_to_cluster(sbi, le64_to_cpu(attr_b->nres.alloc_size));
+	evcn = le64_to_cpu(attr->nres.evcn);
  
-			if (ni_load_mi(ni, le, &mi)) {
-				attr = NULL;
-				goto out;
-			}
+	if (end < next_svcn)
+		end = next_svcn;
+	while (end > evcn) {
+		/* Remove segment [svcn : evcn). */
+		mi_remove_attr(NULL, mi, attr);
  
-			attr = mi_find_attr(mi, NULL, ATTR_DATA, NULL, 0,
-					    &le->id);
-			if (!attr) {
-				err = -EINVAL;
-				goto out;
-			}
-			svcn = le64_to_cpu(attr->nres.svcn);
-			evcn = le64_to_cpu(attr->nres.evcn);
+		if (!al_remove_le(ni, le)) {
+			err = -EINVAL;
+			goto out;
  		}
  
-		if (end < svcn)
-			end = svcn;
+		if (evcn + 1 >= alloc) {
+			/* Last attribute segment. */
+			evcn1 = evcn + 1;
+			goto ins_ext;
+		}
  
-		err = attr_load_runs(attr, ni, run, &end);
-		if (err)
+		if (ni_load_mi(ni, le, &mi)) {
+			attr = NULL;
  			goto out;
+		}
  
-		evcn1 = evcn + 1;
-		attr->nres.svcn = cpu_to_le64(next_svcn);
-		err = mi_pack_runs(mi, attr, run, evcn1 - next_svcn);
-		if (err)
+		attr = mi_find_attr(mi, NULL, ATTR_DATA, NULL, 0, &le->id);
+		if (!attr) {
+			err = -EINVAL;
  			goto out;
+		}
+		svcn = le64_to_cpu(attr->nres.svcn);
+		evcn = le64_to_cpu(attr->nres.evcn);
+	}
  
-		le->vcn = cpu_to_le64(next_svcn);
-		ni->attr_list.dirty = true;
-		mi->dirty = true;
+	if (end < svcn)
+		end = svcn;
+
+	err = attr_load_runs(attr, ni, run, &end);
+	if (err)
+		goto out;
+
+	evcn1 = evcn + 1;
+	attr->nres.svcn = cpu_to_le64(next_svcn);
+	err = mi_pack_runs(mi, attr, run, evcn1 - next_svcn);
+	if (err)
+		goto out;
+
+	le->vcn = cpu_to_le64(next_svcn);
+	ni->attr_list.dirty = true;
+	mi->dirty = true;
+	next_svcn = le64_to_cpu(attr->nres.evcn) + 1;
  
-		next_svcn = le64_to_cpu(attr->nres.evcn) + 1;
-	}
  ins_ext:
  	if (evcn1 > next_svcn) {
  		err = ni_insert_nonresident(ni, ATTR_DATA, NULL, 0, run,
@@ -1170,10 +1195,26 @@ int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
  ok:
  	run_truncate_around(run, vcn);
  out:
+	if (err && step > 1) {
+		/* Too complex to restore. */
+		_ntfs_bad_inode(&ni->vfs_inode);
+	}
  	up_write(&ni->file.run_lock);
  	ni_unlock(ni);
  
  	return err;
+
+undo1:
+	/* Undo step1. */
+	attr_b->nres.total_size = cpu_to_le64(total_size0);
+	inode_set_bytes(&ni->vfs_inode, total_size0);
+
+	if (run_deallocate_ex(sbi, run, vcn, alen, NULL, false) ||
+	    !run_add_entry(run, vcn, SPARSE_LCN, alen, false) ||
+	    mi_pack_runs(mi, attr, run, max(end, evcn1) - svcn)) {
+		_ntfs_bad_inode(&ni->vfs_inode);
+	}
+	goto out;
  }
  
  int attr_data_read_resident(struct ntfs_inode *ni, struct page *page)
diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 3fe2de74eeaf..b56ffb4951cc 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -419,6 +419,39 @@ int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
  	return err;
  }
  
+/*
+ * ntfs_check_for_free_space
+ *
+ * Check if it is possible to allocate 'clen' clusters and 'mlen' Mft records
+ */
+bool ntfs_check_for_free_space(struct ntfs_sb_info *sbi, CLST clen, CLST mlen)
+{
+	size_t free, zlen, avail;
+	struct wnd_bitmap *wnd;
+
+	wnd = &sbi->used.bitmap;
+	down_read_nested(&wnd->rw_lock, BITMAP_MUTEX_CLUSTERS);
+	free = wnd_zeroes(wnd);
+	zlen = wnd_zone_len(wnd);
+	up_read(&wnd->rw_lock);
+
+	if (free < zlen + clen)
+		return false;
+
+	avail = free - (zlen + clen);
+
+	wnd = &sbi->mft.bitmap;
+	down_read_nested(&wnd->rw_lock, BITMAP_MUTEX_MFT);
+	free = wnd_zeroes(wnd);
+	zlen = wnd_zone_len(wnd);
+	up_read(&wnd->rw_lock);
+
+	if (free >= zlen + mlen)
+		return true;
+
+	return avail >= bytes_to_cluster(sbi, mlen << sbi->record_bits);
+}
+
  /*
   * ntfs_extend_mft - Allocate additional MFT records.
   *
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 5fad93a2c3fd..d73d1c837ba7 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -587,6 +587,7 @@ int ntfs_loadlog_and_replay(struct ntfs_inode *ni, struct ntfs_sb_info *sbi);
  int ntfs_look_for_free_space(struct ntfs_sb_info *sbi, CLST lcn, CLST len,
  			     CLST *new_lcn, CLST *new_len,
  			     enum ALLOCATE_OPT opt);
+bool ntfs_check_for_free_space(struct ntfs_sb_info *sbi, CLST clen, CLST mlen);
  int ntfs_look_free_mft(struct ntfs_sb_info *sbi, CLST *rno, bool mft,
  		       struct ntfs_inode *ni, struct mft_inode **mi);
  void ntfs_mark_rec_free(struct ntfs_sb_info *sbi, CLST rno, bool is_mft);
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/14] fs/ntfs3: Correct ntfs_check_for_free_space
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (6 preceding siblings ...)
  2022-10-28 17:04 ` [PATCH 07/14] fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block Konstantin Komarov
@ 2022-10-28 17:05 ` Konstantin Komarov
  2022-10-28 17:05 ` [PATCH 09/14] fs/ntfs3: Check fields while reading Konstantin Komarov
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:05 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

zlen in some cases was bigger than correct value.

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

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index b56ffb4951cc..e5a1f4df0397 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -432,7 +432,7 @@ bool ntfs_check_for_free_space(struct ntfs_sb_info *sbi, CLST clen, CLST mlen)
  	wnd = &sbi->used.bitmap;
  	down_read_nested(&wnd->rw_lock, BITMAP_MUTEX_CLUSTERS);
  	free = wnd_zeroes(wnd);
-	zlen = wnd_zone_len(wnd);
+	zlen = min_t(size_t, NTFS_MIN_MFT_ZONE, wnd_zone_len(wnd));
  	up_read(&wnd->rw_lock);
  
  	if (free < zlen + clen)
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/14] fs/ntfs3: Check fields while reading
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (7 preceding siblings ...)
  2022-10-28 17:05 ` [PATCH 08/14] fs/ntfs3: Correct ntfs_check_for_free_space Konstantin Komarov
@ 2022-10-28 17:05 ` Konstantin Komarov
  2023-06-19  9:41   ` Lee Jones
  2022-10-28 17:06 ` [PATCH 10/14] fs/ntfs3: Fix incorrect if in ntfs_set_acl_ex Konstantin Komarov
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:05 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Added new functions index_hdr_check and index_buf_check.
Now we check all stuff for correctness while reading from disk.
Also fixed bug with stale nfs data.

Reported-by: van fantasy <g1042620637@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/index.c   |  84 ++++++++++++++++++++++++++++++----
  fs/ntfs3/inode.c   |  18 ++++----
  fs/ntfs3/ntfs_fs.h |   4 +-
  fs/ntfs3/run.c     |   7 ++-
  fs/ntfs3/xattr.c   | 109 +++++++++++++++++++++++++++++----------------
  5 files changed, 164 insertions(+), 58 deletions(-)

diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 35369ae5c438..51ab75954640 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -605,11 +605,58 @@ static const struct NTFS_DE *hdr_insert_head(struct INDEX_HDR *hdr,
  	return e;
  }
  
+/*
+ * index_hdr_check
+ *
+ * return true if INDEX_HDR is valid
+ */
+static bool index_hdr_check(const struct INDEX_HDR *hdr, u32 bytes)
+{
+	u32 end = le32_to_cpu(hdr->used);
+	u32 tot = le32_to_cpu(hdr->total);
+	u32 off = le32_to_cpu(hdr->de_off);
+
+	if (!IS_ALIGNED(off, 8) || tot > bytes || end > tot ||
+	    off + sizeof(struct NTFS_DE) > end) {
+		/* incorrect index buffer. */
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * index_buf_check
+ *
+ * return true if INDEX_BUFFER seems is valid
+ */
+static bool index_buf_check(const struct INDEX_BUFFER *ib, u32 bytes,
+			    const CLST *vbn)
+{
+	const struct NTFS_RECORD_HEADER *rhdr = &ib->rhdr;
+	u16 fo = le16_to_cpu(rhdr->fix_off);
+	u16 fn = le16_to_cpu(rhdr->fix_num);
+
+	if (bytes <= offsetof(struct INDEX_BUFFER, ihdr) ||
+	    rhdr->sign != NTFS_INDX_SIGNATURE ||
+	    fo < sizeof(struct INDEX_BUFFER)
+	    /* Check index buffer vbn. */
+	    || (vbn && *vbn != le64_to_cpu(ib->vbn)) || (fo % sizeof(short)) ||
+	    fo + fn * sizeof(short) >= bytes ||
+	    fn != ((bytes >> SECTOR_SHIFT) + 1)) {
+		/* incorrect index buffer. */
+		return false;
+	}
+
+	return index_hdr_check(&ib->ihdr,
+			       bytes - offsetof(struct INDEX_BUFFER, ihdr));
+}
+
  void fnd_clear(struct ntfs_fnd *fnd)
  {
  	int i;
  
-	for (i = 0; i < fnd->level; i++) {
+	for (i = fnd->level - 1; i >= 0; i--) {
  		struct indx_node *n = fnd->nodes[i];
  
  		if (!n)
@@ -819,9 +866,16 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
  	u32 t32;
  	const struct INDEX_ROOT *root = resident_data(attr);
  
+	t32 = le32_to_cpu(attr->res.data_size);
+	if (t32 <= offsetof(struct INDEX_ROOT, ihdr) ||
+	    !index_hdr_check(&root->ihdr,
+			     t32 - offsetof(struct INDEX_ROOT, ihdr))) {
+		goto out;
+	}
+
  	/* Check root fields. */
  	if (!root->index_block_clst)
-		return -EINVAL;
+		goto out;
  
  	indx->type = type;
  	indx->idx2vbn_bits = __ffs(root->index_block_clst);
@@ -833,19 +887,19 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
  	if (t32 < sbi->cluster_size) {
  		/* Index record is smaller than a cluster, use 512 blocks. */
  		if (t32 != root->index_block_clst * SECTOR_SIZE)
-			return -EINVAL;
+			goto out;
  
  		/* Check alignment to a cluster. */
  		if ((sbi->cluster_size >> SECTOR_SHIFT) &
  		    (root->index_block_clst - 1)) {
-			return -EINVAL;
+			goto out;
  		}
  
  		indx->vbn2vbo_bits = SECTOR_SHIFT;
  	} else {
  		/* Index record must be a multiple of cluster size. */
  		if (t32 != root->index_block_clst << sbi->cluster_bits)
-			return -EINVAL;
+			goto out;
  
  		indx->vbn2vbo_bits = sbi->cluster_bits;
  	}
@@ -853,7 +907,14 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
  	init_rwsem(&indx->run_lock);
  
  	indx->cmp = get_cmp_func(root);
-	return indx->cmp ? 0 : -EINVAL;
+	if (!indx->cmp)
+		goto out;
+
+	return 0;
+
+out:
+	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
+	return -EINVAL;
  }
  
  static struct indx_node *indx_new(struct ntfs_index *indx,
@@ -1011,6 +1072,13 @@ int indx_read(struct ntfs_index *indx, struct ntfs_inode *ni, CLST vbn,
  		goto out;
  
  ok:
+	if (!index_buf_check(ib, bytes, &vbn)) {
+		ntfs_inode_err(&ni->vfs_inode, "directory corrupted");
+		ntfs_set_state(ni->mi.sbi, NTFS_DIRTY_ERROR);
+		err = -EINVAL;
+		goto out;
+	}
+
  	if (err == -E_NTFS_FIXUP) {
  		ntfs_write_bh(ni->mi.sbi, &ib->rhdr, &in->nb, 0);
  		err = 0;
@@ -1601,9 +1669,9 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni,
  
  	if (err) {
  		/* Restore root. */
-		if (mi_resize_attr(mi, attr, -ds_root))
+		if (mi_resize_attr(mi, attr, -ds_root)) {
  			memcpy(attr, a_root, asize);
-		else {
+		} else {
  			/* Bug? */
  			ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
  		}
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 78ec3e6bbf67..719cf6fbb5ed 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -81,7 +81,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
  			 le16_to_cpu(ref->seq), le16_to_cpu(rec->seq));
  		goto out;
  	} else if (!is_rec_inuse(rec)) {
-		err = -EINVAL;
+		err = -ESTALE;
  		ntfs_err(sb, "Inode r=%x is not in use!", (u32)ino);
  		goto out;
  	}
@@ -92,8 +92,10 @@ static struct inode *ntfs_read_mft(struct inode *inode,
  		goto out;
  	}
  
-	if (!is_rec_base(rec))
-		goto Ok;
+	if (!is_rec_base(rec)) {
+		err = -EINVAL;
+		goto out;
+	}
  
  	/* Record should contain $I30 root. */
  	is_dir = rec->flags & RECORD_FLAG_DIR;
@@ -465,7 +467,6 @@ static struct inode *ntfs_read_mft(struct inode *inode,
  		inode->i_flags |= S_NOSEC;
  	}
  
-Ok:
  	if (ino == MFT_REC_MFT && !sb->s_root)
  		sbi->mft.ni = NULL;
  
@@ -519,6 +520,9 @@ struct inode *ntfs_iget5(struct super_block *sb, const struct MFT_REF *ref,
  		_ntfs_bad_inode(inode);
  	}
  
+	if (IS_ERR(inode) && name)
+		ntfs_set_state(sb->s_fs_info, NTFS_DIRTY_ERROR);
+
  	return inode;
  }
  
@@ -1652,10 +1656,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
  		ntfs_remove_reparse(sbi, IO_REPARSE_TAG_SYMLINK, &new_de->ref);
  
  out5:
-	if (S_ISDIR(mode) || run_is_empty(&ni->file.run))
-		goto out4;
-
-	run_deallocate(sbi, &ni->file.run, false);
+	if (!S_ISDIR(mode))
+		run_deallocate(sbi, &ni->file.run, false);
  
  out4:
  	clear_rec_inuse(rec);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index d73d1c837ba7..c9b8a6f1ba0b 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -795,12 +795,12 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
  	     u32 run_buf_size, CLST *packed_vcns);
  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
-	       u32 run_buf_size);
+	       int run_buf_size);
  
  #ifdef NTFS3_CHECK_FREE_CLST
  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
-		  u32 run_buf_size);
+		  int run_buf_size);
  #else
  #define run_unpack_ex run_unpack
  #endif
diff --git a/fs/ntfs3/run.c b/fs/ntfs3/run.c
index aaaa0d3d35a2..12d8682f33b5 100644
--- a/fs/ntfs3/run.c
+++ b/fs/ntfs3/run.c
@@ -919,12 +919,15 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
   */
  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
-	       u32 run_buf_size)
+	       int run_buf_size)
  {
  	u64 prev_lcn, vcn64, lcn, next_vcn;
  	const u8 *run_last, *run_0;
  	bool is_mft = ino == MFT_REC_MFT;
  
+	if (run_buf_size < 0)
+		return -EINVAL;
+
  	/* Check for empty. */
  	if (evcn + 1 == svcn)
  		return 0;
@@ -1046,7 +1049,7 @@ int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
   */
  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
-		  u32 run_buf_size)
+		  int run_buf_size)
  {
  	int ret, err;
  	CLST next_vcn, lcn, len;
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index aeee5fb12092..385c50831a8d 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -42,28 +42,26 @@ static inline size_t packed_ea_size(const struct EA_FULL *ea)
   * Assume there is at least one xattr in the list.
   */
  static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
-			   const char *name, u8 name_len, u32 *off)
+			   const char *name, u8 name_len, u32 *off, u32 *ea_sz)
  {
-	*off = 0;
+	u32 ea_size;
  
-	if (!ea_all || !bytes)
+	*off = 0;
+	if (!ea_all)
  		return false;
  
-	for (;;) {
+	for (; *off < bytes; *off += ea_size) {
  		const struct EA_FULL *ea = Add2Ptr(ea_all, *off);
-		u32 next_off = *off + unpacked_ea_size(ea);
-
-		if (next_off > bytes)
-			return false;
-
+		ea_size = unpacked_ea_size(ea);
  		if (ea->name_len == name_len &&
-		    !memcmp(ea->name, name, name_len))
+		    !memcmp(ea->name, name, name_len)) {
+			if (ea_sz)
+				*ea_sz = ea_size;
  			return true;
-
-		*off = next_off;
-		if (next_off >= bytes)
-			return false;
+		}
  	}
+
+	return false;
  }
  
  /*
@@ -74,12 +72,12 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
  static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
  			size_t add_bytes, const struct EA_INFO **info)
  {
-	int err;
+	int err = -EINVAL;
  	struct ntfs_sb_info *sbi = ni->mi.sbi;
  	struct ATTR_LIST_ENTRY *le = NULL;
  	struct ATTRIB *attr_info, *attr_ea;
  	void *ea_p;
-	u32 size;
+	u32 size, off, ea_size;
  
  	static_assert(le32_to_cpu(ATTR_EA_INFO) < le32_to_cpu(ATTR_EA));
  
@@ -96,24 +94,31 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
  
  	*info = resident_data_ex(attr_info, sizeof(struct EA_INFO));
  	if (!*info)
-		return -EINVAL;
+		goto out;
  
  	/* Check Ea limit. */
  	size = le32_to_cpu((*info)->size);
-	if (size > sbi->ea_max_size)
-		return -EFBIG;
+	if (size > sbi->ea_max_size) {
+		err = -EFBIG;
+		goto out;
+	}
+
+	if (attr_size(attr_ea) > sbi->ea_max_size) {
+		err = -EFBIG;
+		goto out;
+	}
  
-	if (attr_size(attr_ea) > sbi->ea_max_size)
-		return -EFBIG;
+	if (!size) {
+		/* EA info persists, but xattr is empty. Looks like EA problem. */
+		goto out;
+	}
  
  	/* Allocate memory for packed Ea. */
  	ea_p = kmalloc(size_add(size, add_bytes), GFP_NOFS);
  	if (!ea_p)
  		return -ENOMEM;
  
-	if (!size) {
-		/* EA info persists, but xattr is empty. Looks like EA problem. */
-	} else if (attr_ea->non_res) {
+	if (attr_ea->non_res) {
  		struct runs_tree run;
  
  		run_init(&run);
@@ -124,24 +129,52 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
  		run_close(&run);
  
  		if (err)
-			goto out;
+			goto out1;
  	} else {
  		void *p = resident_data_ex(attr_ea, size);
  
-		if (!p) {
-			err = -EINVAL;
-			goto out;
-		}
+		if (!p)
+			goto out1;
  		memcpy(ea_p, p, size);
  	}
  
  	memset(Add2Ptr(ea_p, size), 0, add_bytes);
+
+	/* Check all attributes for consistency. */
+	for (off = 0; off < size; off += ea_size) {
+		const struct EA_FULL *ef = Add2Ptr(ea_p, off);
+		u32 bytes = size - off;
+
+		/* Check if we can use field ea->size. */
+		if (bytes < sizeof(ef->size))
+			goto out1;
+
+		if (ef->size) {
+			ea_size = le32_to_cpu(ef->size);
+			if (ea_size > bytes)
+				goto out1;
+			continue;
+		}
+
+		/* Check if we can use fields ef->name_len and ef->elength. */
+		if (bytes < offsetof(struct EA_FULL, name))
+			goto out1;
+
+		ea_size = ALIGN(struct_size(ef, name,
+					    1 + ef->name_len +
+						    le16_to_cpu(ef->elength)),
+				4);
+		if (ea_size > bytes)
+			goto out1;
+	}
+
  	*ea = ea_p;
  	return 0;
  
-out:
+out1:
  	kfree(ea_p);
-	*ea = NULL;
+out:
+	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
  	return err;
  }
  
@@ -163,6 +196,7 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, char *buffer,
  	const struct EA_FULL *ea;
  	u32 off, size;
  	int err;
+	int ea_size;
  	size_t ret;
  
  	err = ntfs_read_ea(ni, &ea_all, 0, &info);
@@ -175,8 +209,9 @@ 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 < size; off += unpacked_ea_size(ea)) {
+	for (ret = 0, off = 0; off < size; off += ea_size) {
  		ea = Add2Ptr(ea_all, off);
+		ea_size = unpacked_ea_size(ea);
  
  		if (buffer) {
  			if (ret + ea->name_len + 1 > bytes_per_buffer) {
@@ -227,7 +262,8 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
  		goto out;
  
  	/* Enumerate all xattrs. */
-	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off)) {
+	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off,
+		     NULL)) {
  		err = -ENODATA;
  		goto out;
  	}
@@ -269,7 +305,7 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
  	struct EA_FULL *new_ea;
  	struct EA_FULL *ea_all = NULL;
  	size_t add, new_pack;
-	u32 off, size;
+	u32 off, size, ea_sz;
  	__le16 size_pack;
  	struct ATTRIB *attr;
  	struct ATTR_LIST_ENTRY *le;
@@ -304,9 +340,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
  		size_pack = ea_info.size_pack;
  	}
  
-	if (info && find_ea(ea_all, size, name, name_len, &off)) {
+	if (info && find_ea(ea_all, size, name, name_len, &off, &ea_sz)) {
  		struct EA_FULL *ea;
-		size_t ea_sz;
  
  		if (flags & XATTR_CREATE) {
  			err = -EEXIST;
@@ -329,8 +364,6 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
  		if (ea->flags & FILE_NEED_EA)
  			le16_add_cpu(&ea_info.count, -1);
  
-		ea_sz = unpacked_ea_size(ea);
-
  		le16_add_cpu(&ea_info.size_pack, 0 - packed_ea_size(ea));
  
  		memmove(ea, Add2Ptr(ea, ea_sz), size - off - ea_sz);
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/14] fs/ntfs3: Fix incorrect if in ntfs_set_acl_ex
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (8 preceding siblings ...)
  2022-10-28 17:05 ` [PATCH 09/14] fs/ntfs3: Check fields while reading Konstantin Komarov
@ 2022-10-28 17:06 ` Konstantin Komarov
  2022-10-28 17:06 ` [PATCH 11/14] fs/ntfs3: Use ALIGN kernel macro Konstantin Komarov
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:06 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

We need to update ctime too with mode.
Fixes xfstest generic/307

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

diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 385c50831a8d..bead9c3059ce 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -637,10 +637,9 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
  		err = 0; /* Removing non existed xattr. */
  	if (!err) {
  		set_cached_acl(inode, type, acl);
-		if (inode->i_mode != mode) {
-			inode->i_mode = mode;
-			mark_inode_dirty(inode);
-		}
+		inode->i_mode = mode;
+		inode->i_ctime = current_time(inode);
+		mark_inode_dirty(inode);
  	}
  
  out:
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 11/14] fs/ntfs3: Use ALIGN kernel macro
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (9 preceding siblings ...)
  2022-10-28 17:06 ` [PATCH 10/14] fs/ntfs3: Fix incorrect if in ntfs_set_acl_ex Konstantin Komarov
@ 2022-10-28 17:06 ` Konstantin Komarov
  2022-10-28 17:07 ` [PATCH 12/14] fs/ntfs3: Fix wrong if in hdr_first_de Konstantin Komarov
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:06 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

This way code will be more readable.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/fsntfs.c  | 2 +-
  fs/ntfs3/ntfs.h    | 1 -
  fs/ntfs3/ntfs_fs.h | 2 ++
  3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index e5a1f4df0397..1e01a3e8e01e 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -479,7 +479,7 @@ static int ntfs_extend_mft(struct ntfs_sb_info *sbi)
  	struct ATTRIB *attr;
  	struct wnd_bitmap *wnd = &sbi->mft.bitmap;
  
-	new_mft_total = (wnd->nbits + MFT_INCREASE_CHUNK + 127) & (CLST)~127;
+	new_mft_total = ALIGN(wnd->nbits + NTFS_MFT_INCREASE_STEP, 128);
  	new_mft_bytes = (u64)new_mft_total << sbi->record_bits;
  
  	/* Step 1: Resize $MFT::DATA. */
diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 9cc396b117bf..9f764bf4ed0a 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -84,7 +84,6 @@ typedef u32 CLST;
  
  #define COMPRESSION_UNIT     4
  #define COMPRESS_MAX_CLUSTER 0x1000
-#define MFT_INCREASE_CHUNK   1024
  
  enum RECORD_NUM {
  	MFT_REC_MFT		= 0,
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index c9b8a6f1ba0b..bc02cfb344f9 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -198,6 +198,8 @@ struct ntfs_index {
  
  /* Minimum MFT zone. */
  #define NTFS_MIN_MFT_ZONE 100
+/* Step to increase the MFT. */
+#define NTFS_MFT_INCREASE_STEP 1024
  
  /* Ntfs file system in-core superblock data. */
  struct ntfs_sb_info {
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 12/14] fs/ntfs3: Fix wrong if in hdr_first_de
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (10 preceding siblings ...)
  2022-10-28 17:06 ` [PATCH 11/14] fs/ntfs3: Use ALIGN kernel macro Konstantin Komarov
@ 2022-10-28 17:07 ` Konstantin Komarov
  2022-10-28 17:07 ` [PATCH 13/14] fs/ntfs3: Improve checking of bad clusters Konstantin Komarov
  2022-10-28 17:08 ` [PATCH 14/14] fs/ntfs3: Make if more readable Konstantin Komarov
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:07 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

We need to check used bytes instead of total.

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

diff --git a/fs/ntfs3/ntfs.h b/fs/ntfs3/ntfs.h
index 9f764bf4ed0a..86ea1826d099 100644
--- a/fs/ntfs3/ntfs.h
+++ b/fs/ntfs3/ntfs.h
@@ -714,12 +714,13 @@ static inline struct NTFS_DE *hdr_first_de(const struct INDEX_HDR *hdr)
  {
  	u32 de_off = le32_to_cpu(hdr->de_off);
  	u32 used = le32_to_cpu(hdr->used);
-	struct NTFS_DE *e = Add2Ptr(hdr, de_off);
+	struct NTFS_DE *e;
  	u16 esize;
  
-	if (de_off >= used || de_off >= le32_to_cpu(hdr->total))
+	if (de_off >= used || de_off + sizeof(struct NTFS_DE) > used )
  		return NULL;
  
+	e = Add2Ptr(hdr, de_off);
  	esize = le16_to_cpu(e->size);
  	if (esize < sizeof(struct NTFS_DE) || de_off + esize > used)
  		return NULL;
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 13/14] fs/ntfs3: Improve checking of bad clusters
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (11 preceding siblings ...)
  2022-10-28 17:07 ` [PATCH 12/14] fs/ntfs3: Fix wrong if in hdr_first_de Konstantin Komarov
@ 2022-10-28 17:07 ` Konstantin Komarov
  2022-10-28 17:08 ` [PATCH 14/14] fs/ntfs3: Make if more readable Konstantin Komarov
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:07 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Added new function wnd_set_used_safe.
Load $BadClus before $AttrDef instead of before $Bitmap.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
  fs/ntfs3/bitmap.c  | 38 +++++++++++++++++++++++++++
  fs/ntfs3/ntfs_fs.h |  2 ++
  fs/ntfs3/run.c     | 21 ++-------------
  fs/ntfs3/super.c   | 64 ++++++++++++++++++++++++++++------------------
  4 files changed, 81 insertions(+), 44 deletions(-)

diff --git a/fs/ntfs3/bitmap.c b/fs/ntfs3/bitmap.c
index 6e68887597ac..c86a76dd44b9 100644
--- a/fs/ntfs3/bitmap.c
+++ b/fs/ntfs3/bitmap.c
@@ -800,6 +800,44 @@ int wnd_set_used(struct wnd_bitmap *wnd, size_t bit, size_t bits)
  	return err;
  }
  
+/*
+ * wnd_set_used_safe - Mark the bits range from bit to bit + bits as used.
+ *
+ * Unlikely wnd_set_used/wnd_set_free this function is not full trusted.
+ * It scans every bit in bitmap and marks free bit as used.
+ * @done - how many bits were marked as used.
+ *
+ * NOTE: normally *done should be 0.
+ */
+int wnd_set_used_safe(struct wnd_bitmap *wnd, size_t bit, size_t bits,
+		      size_t *done)
+{
+	size_t i, from = 0, len = 0;
+	int err = 0;
+
+	*done = 0;
+	for (i = 0; i < bits; i++) {
+		if (wnd_is_free(wnd, bit + i, 1)) {
+			if (!len)
+				from = bit + i;
+			len += 1;
+		} else if (len) {
+			err = wnd_set_used(wnd, from, len);
+			*done += len;
+			len = 0;
+			if (err)
+				break;
+		}
+	}
+
+	if (len) {
+		/* last fragment. */
+		err = wnd_set_used(wnd, from, len);
+		*done += len;
+	}
+	return err;
+}
+
  /*
   * wnd_is_free_hlp
   *
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index bc02cfb344f9..2f55993a716c 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -826,6 +826,8 @@ static inline size_t wnd_zeroes(const struct wnd_bitmap *wnd)
  int wnd_init(struct wnd_bitmap *wnd, struct super_block *sb, size_t nbits);
  int wnd_set_free(struct wnd_bitmap *wnd, size_t bit, size_t bits);
  int wnd_set_used(struct wnd_bitmap *wnd, size_t bit, size_t bits);
+int wnd_set_used_safe(struct wnd_bitmap *wnd, size_t bit, size_t bits,
+		      size_t *done);
  bool wnd_is_free(struct wnd_bitmap *wnd, size_t bit, size_t bits);
  bool wnd_is_used(struct wnd_bitmap *wnd, size_t bit, size_t bits);
  
diff --git a/fs/ntfs3/run.c b/fs/ntfs3/run.c
index 12d8682f33b5..a5af71cd8d14 100644
--- a/fs/ntfs3/run.c
+++ b/fs/ntfs3/run.c
@@ -1096,25 +1096,8 @@ int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
  
  		if (down_write_trylock(&wnd->rw_lock)) {
  			/* Mark all zero bits as used in range [lcn, lcn+len). */
-			CLST i, lcn_f = 0, len_f = 0;
-
-			err = 0;
-			for (i = 0; i < len; i++) {
-				if (wnd_is_free(wnd, lcn + i, 1)) {
-					if (!len_f)
-						lcn_f = lcn + i;
-					len_f += 1;
-				} else if (len_f) {
-					err = wnd_set_used(wnd, lcn_f, len_f);
-					len_f = 0;
-					if (err)
-						break;
-				}
-			}
-
-			if (len_f)
-				err = wnd_set_used(wnd, lcn_f, len_f);
-
+			size_t done;
+			err = wnd_set_used_safe(wnd, lcn, len, &done);
  			up_write(&wnd->rw_lock);
  			if (err)
  				return err;
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 1e2c04e48f98..d8ba6724adf1 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -921,7 +921,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
  	struct block_device *bdev = sb->s_bdev;
  	struct inode *inode;
  	struct ntfs_inode *ni;
-	size_t i, tt;
+	size_t i, tt, bad_len, bad_frags;
  	CLST vcn, lcn, len;
  	struct ATTRIB *attr;
  	const struct VOLUME_INFO *info;
@@ -1091,30 +1091,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
  
  	sbi->mft.ni = ni;
  
-	/* Load $BadClus. */
-	ref.low = cpu_to_le32(MFT_REC_BADCLUST);
-	ref.seq = cpu_to_le16(MFT_REC_BADCLUST);
-	inode = ntfs_iget5(sb, &ref, &NAME_BADCLUS);
-	if (IS_ERR(inode)) {
-		ntfs_err(sb, "Failed to load $BadClus.");
-		err = PTR_ERR(inode);
-		goto out;
-	}
-
-	ni = ntfs_i(inode);
-
-	for (i = 0; run_get_entry(&ni->file.run, i, &vcn, &lcn, &len); i++) {
-		if (lcn == SPARSE_LCN)
-			continue;
-
-		if (!sbi->bad_clusters)
-			ntfs_notice(sb, "Volume contains bad blocks");
-
-		sbi->bad_clusters += len;
-	}
-
-	iput(inode);
-
  	/* Load $Bitmap. */
  	ref.low = cpu_to_le32(MFT_REC_BITMAP);
  	ref.seq = cpu_to_le16(MFT_REC_BITMAP);
@@ -1152,6 +1128,44 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
  	if (err)
  		goto out;
  
+	/* Load $BadClus. */
+	ref.low = cpu_to_le32(MFT_REC_BADCLUST);
+	ref.seq = cpu_to_le16(MFT_REC_BADCLUST);
+	inode = ntfs_iget5(sb, &ref, &NAME_BADCLUS);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
+		ntfs_err(sb, "Failed to load $BadClus (%d).", err);
+		goto out;
+	}
+
+	ni = ntfs_i(inode);
+	bad_len = bad_frags = 0;
+	for (i = 0; run_get_entry(&ni->file.run, i, &vcn, &lcn, &len); i++) {
+		if (lcn == SPARSE_LCN)
+			continue;
+
+		bad_len += len;
+		bad_frags += 1;
+		if (sb_rdonly(sb))
+			continue;
+
+		if (wnd_set_used_safe(&sbi->used.bitmap, lcn, len, &tt) || tt) {
+			/* Bad blocks marked as free in bitmap. */
+			ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
+		}
+	}
+	if (bad_len) {
+		/*
+		 * Notice about bad blocks.
+		 * In normal cases these blocks are marked as used in bitmap.
+		 * And we never allocate space in it.
+		 */
+		ntfs_notice(sb,
+			    "Volume contains %zu bad blocks in %zu fragments.",
+			    bad_len, bad_frags);
+	}
+	iput(inode);
+
  	/* Load $AttrDef. */
  	ref.low = cpu_to_le32(MFT_REC_ATTR);
  	ref.seq = cpu_to_le16(MFT_REC_ATTR);
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 14/14] fs/ntfs3: Make if more readable
  2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
                   ` (12 preceding siblings ...)
  2022-10-28 17:07 ` [PATCH 13/14] fs/ntfs3: Improve checking of bad clusters Konstantin Komarov
@ 2022-10-28 17:08 ` Konstantin Komarov
  13 siblings, 0 replies; 17+ messages in thread
From: Konstantin Komarov @ 2022-10-28 17:08 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

This way it looks better.

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

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index a952cd7aa7a4..defce6a5c8e1 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -265,10 +265,9 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, struct ATTRIB *attr)
  		if (t16 + t32 > asize)
  			return NULL;
  
-		if (attr->name_len &&
-		    le16_to_cpu(attr->name_off) + sizeof(short) * attr->name_len > t16) {
+		t32 = sizeof(short) * attr->name_len;
+		if (t32 && le16_to_cpu(attr->name_off) + t32 > t16)
  			return NULL;
-		}
  
  		return attr;
  	}
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/14] fs/ntfs3: Check fields while reading
  2022-10-28 17:05 ` [PATCH 09/14] fs/ntfs3: Check fields while reading Konstantin Komarov
@ 2023-06-19  9:41   ` Lee Jones
  2023-06-27 13:49     ` Lee Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2023-06-19  9:41 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel

On Fri, 28 Oct 2022, Konstantin Komarov wrote:

> Added new functions index_hdr_check and index_buf_check.
> Now we check all stuff for correctness while reading from disk.
> Also fixed bug with stale nfs data.
> 
> Reported-by: van fantasy <g1042620637@gmail.com>
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> ---
>  fs/ntfs3/index.c   |  84 ++++++++++++++++++++++++++++++----
>  fs/ntfs3/inode.c   |  18 ++++----
>  fs/ntfs3/ntfs_fs.h |   4 +-
>  fs/ntfs3/run.c     |   7 ++-
>  fs/ntfs3/xattr.c   | 109 +++++++++++++++++++++++++++++----------------
>  5 files changed, 164 insertions(+), 58 deletions(-)

It's not clear to me what route this patch took into Mainline [0].  My
guess it was authored by and then merged by the maintainer after no one
raised any review comments.

It does appear that this particular patch was identified as the fix for
a published CVE however [1], and with no Fixes: tag I'm concerned that
the Stable AUTOSEL bot may miss this.  With that in mind, would you mind
submitting to the relevant Stable branches please?  [2] contains all of
the relevant information if you're not 100% certain of the process.

Thank you.

[0] 0e8235d28f3a0 fs/ntfs3: Check fields while reading
[1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-48502
[2] Documentation/process/stable-kernel-rules.rst

> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index 35369ae5c438..51ab75954640 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -605,11 +605,58 @@ static const struct NTFS_DE *hdr_insert_head(struct INDEX_HDR *hdr,
>  	return e;
>  }
> +/*
> + * index_hdr_check
> + *
> + * return true if INDEX_HDR is valid
> + */
> +static bool index_hdr_check(const struct INDEX_HDR *hdr, u32 bytes)
> +{
> +	u32 end = le32_to_cpu(hdr->used);
> +	u32 tot = le32_to_cpu(hdr->total);
> +	u32 off = le32_to_cpu(hdr->de_off);
> +
> +	if (!IS_ALIGNED(off, 8) || tot > bytes || end > tot ||
> +	    off + sizeof(struct NTFS_DE) > end) {
> +		/* incorrect index buffer. */
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * index_buf_check
> + *
> + * return true if INDEX_BUFFER seems is valid
> + */
> +static bool index_buf_check(const struct INDEX_BUFFER *ib, u32 bytes,
> +			    const CLST *vbn)
> +{
> +	const struct NTFS_RECORD_HEADER *rhdr = &ib->rhdr;
> +	u16 fo = le16_to_cpu(rhdr->fix_off);
> +	u16 fn = le16_to_cpu(rhdr->fix_num);
> +
> +	if (bytes <= offsetof(struct INDEX_BUFFER, ihdr) ||
> +	    rhdr->sign != NTFS_INDX_SIGNATURE ||
> +	    fo < sizeof(struct INDEX_BUFFER)
> +	    /* Check index buffer vbn. */
> +	    || (vbn && *vbn != le64_to_cpu(ib->vbn)) || (fo % sizeof(short)) ||
> +	    fo + fn * sizeof(short) >= bytes ||
> +	    fn != ((bytes >> SECTOR_SHIFT) + 1)) {
> +		/* incorrect index buffer. */
> +		return false;
> +	}
> +
> +	return index_hdr_check(&ib->ihdr,
> +			       bytes - offsetof(struct INDEX_BUFFER, ihdr));
> +}
> +
>  void fnd_clear(struct ntfs_fnd *fnd)
>  {
>  	int i;
> -	for (i = 0; i < fnd->level; i++) {
> +	for (i = fnd->level - 1; i >= 0; i--) {
>  		struct indx_node *n = fnd->nodes[i];
>  		if (!n)
> @@ -819,9 +866,16 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
>  	u32 t32;
>  	const struct INDEX_ROOT *root = resident_data(attr);
> +	t32 = le32_to_cpu(attr->res.data_size);
> +	if (t32 <= offsetof(struct INDEX_ROOT, ihdr) ||
> +	    !index_hdr_check(&root->ihdr,
> +			     t32 - offsetof(struct INDEX_ROOT, ihdr))) {
> +		goto out;
> +	}
> +
>  	/* Check root fields. */
>  	if (!root->index_block_clst)
> -		return -EINVAL;
> +		goto out;
>  	indx->type = type;
>  	indx->idx2vbn_bits = __ffs(root->index_block_clst);
> @@ -833,19 +887,19 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
>  	if (t32 < sbi->cluster_size) {
>  		/* Index record is smaller than a cluster, use 512 blocks. */
>  		if (t32 != root->index_block_clst * SECTOR_SIZE)
> -			return -EINVAL;
> +			goto out;
>  		/* Check alignment to a cluster. */
>  		if ((sbi->cluster_size >> SECTOR_SHIFT) &
>  		    (root->index_block_clst - 1)) {
> -			return -EINVAL;
> +			goto out;
>  		}
>  		indx->vbn2vbo_bits = SECTOR_SHIFT;
>  	} else {
>  		/* Index record must be a multiple of cluster size. */
>  		if (t32 != root->index_block_clst << sbi->cluster_bits)
> -			return -EINVAL;
> +			goto out;
>  		indx->vbn2vbo_bits = sbi->cluster_bits;
>  	}
> @@ -853,7 +907,14 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
>  	init_rwsem(&indx->run_lock);
>  	indx->cmp = get_cmp_func(root);
> -	return indx->cmp ? 0 : -EINVAL;
> +	if (!indx->cmp)
> +		goto out;
> +
> +	return 0;
> +
> +out:
> +	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
> +	return -EINVAL;
>  }
>  static struct indx_node *indx_new(struct ntfs_index *indx,
> @@ -1011,6 +1072,13 @@ int indx_read(struct ntfs_index *indx, struct ntfs_inode *ni, CLST vbn,
>  		goto out;
>  ok:
> +	if (!index_buf_check(ib, bytes, &vbn)) {
> +		ntfs_inode_err(&ni->vfs_inode, "directory corrupted");
> +		ntfs_set_state(ni->mi.sbi, NTFS_DIRTY_ERROR);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
>  	if (err == -E_NTFS_FIXUP) {
>  		ntfs_write_bh(ni->mi.sbi, &ib->rhdr, &in->nb, 0);
>  		err = 0;
> @@ -1601,9 +1669,9 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni,
>  	if (err) {
>  		/* Restore root. */
> -		if (mi_resize_attr(mi, attr, -ds_root))
> +		if (mi_resize_attr(mi, attr, -ds_root)) {
>  			memcpy(attr, a_root, asize);
> -		else {
> +		} else {
>  			/* Bug? */
>  			ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
>  		}
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 78ec3e6bbf67..719cf6fbb5ed 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -81,7 +81,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>  			 le16_to_cpu(ref->seq), le16_to_cpu(rec->seq));
>  		goto out;
>  	} else if (!is_rec_inuse(rec)) {
> -		err = -EINVAL;
> +		err = -ESTALE;
>  		ntfs_err(sb, "Inode r=%x is not in use!", (u32)ino);
>  		goto out;
>  	}
> @@ -92,8 +92,10 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>  		goto out;
>  	}
> -	if (!is_rec_base(rec))
> -		goto Ok;
> +	if (!is_rec_base(rec)) {
> +		err = -EINVAL;
> +		goto out;
> +	}
>  	/* Record should contain $I30 root. */
>  	is_dir = rec->flags & RECORD_FLAG_DIR;
> @@ -465,7 +467,6 @@ static struct inode *ntfs_read_mft(struct inode *inode,
>  		inode->i_flags |= S_NOSEC;
>  	}
> -Ok:
>  	if (ino == MFT_REC_MFT && !sb->s_root)
>  		sbi->mft.ni = NULL;
> @@ -519,6 +520,9 @@ struct inode *ntfs_iget5(struct super_block *sb, const struct MFT_REF *ref,
>  		_ntfs_bad_inode(inode);
>  	}
> +	if (IS_ERR(inode) && name)
> +		ntfs_set_state(sb->s_fs_info, NTFS_DIRTY_ERROR);
> +
>  	return inode;
>  }
> @@ -1652,10 +1656,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
>  		ntfs_remove_reparse(sbi, IO_REPARSE_TAG_SYMLINK, &new_de->ref);
>  out5:
> -	if (S_ISDIR(mode) || run_is_empty(&ni->file.run))
> -		goto out4;
> -
> -	run_deallocate(sbi, &ni->file.run, false);
> +	if (!S_ISDIR(mode))
> +		run_deallocate(sbi, &ni->file.run, false);
>  out4:
>  	clear_rec_inuse(rec);
> diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> index d73d1c837ba7..c9b8a6f1ba0b 100644
> --- a/fs/ntfs3/ntfs_fs.h
> +++ b/fs/ntfs3/ntfs_fs.h
> @@ -795,12 +795,12 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
>  	     u32 run_buf_size, CLST *packed_vcns);
>  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
>  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> -	       u32 run_buf_size);
> +	       int run_buf_size);
>  #ifdef NTFS3_CHECK_FREE_CLST
>  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
>  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> -		  u32 run_buf_size);
> +		  int run_buf_size);
>  #else
>  #define run_unpack_ex run_unpack
>  #endif
> diff --git a/fs/ntfs3/run.c b/fs/ntfs3/run.c
> index aaaa0d3d35a2..12d8682f33b5 100644
> --- a/fs/ntfs3/run.c
> +++ b/fs/ntfs3/run.c
> @@ -919,12 +919,15 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
>   */
>  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
>  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> -	       u32 run_buf_size)
> +	       int run_buf_size)
>  {
>  	u64 prev_lcn, vcn64, lcn, next_vcn;
>  	const u8 *run_last, *run_0;
>  	bool is_mft = ino == MFT_REC_MFT;
> +	if (run_buf_size < 0)
> +		return -EINVAL;
> +
>  	/* Check for empty. */
>  	if (evcn + 1 == svcn)
>  		return 0;
> @@ -1046,7 +1049,7 @@ int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
>   */
>  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
>  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> -		  u32 run_buf_size)
> +		  int run_buf_size)
>  {
>  	int ret, err;
>  	CLST next_vcn, lcn, len;
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index aeee5fb12092..385c50831a8d 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -42,28 +42,26 @@ static inline size_t packed_ea_size(const struct EA_FULL *ea)
>   * Assume there is at least one xattr in the list.
>   */
>  static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
> -			   const char *name, u8 name_len, u32 *off)
> +			   const char *name, u8 name_len, u32 *off, u32 *ea_sz)
>  {
> -	*off = 0;
> +	u32 ea_size;
> -	if (!ea_all || !bytes)
> +	*off = 0;
> +	if (!ea_all)
>  		return false;
> -	for (;;) {
> +	for (; *off < bytes; *off += ea_size) {
>  		const struct EA_FULL *ea = Add2Ptr(ea_all, *off);
> -		u32 next_off = *off + unpacked_ea_size(ea);
> -
> -		if (next_off > bytes)
> -			return false;
> -
> +		ea_size = unpacked_ea_size(ea);
>  		if (ea->name_len == name_len &&
> -		    !memcmp(ea->name, name, name_len))
> +		    !memcmp(ea->name, name, name_len)) {
> +			if (ea_sz)
> +				*ea_sz = ea_size;
>  			return true;
> -
> -		*off = next_off;
> -		if (next_off >= bytes)
> -			return false;
> +		}
>  	}
> +
> +	return false;
>  }
>  /*
> @@ -74,12 +72,12 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
>  static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
>  			size_t add_bytes, const struct EA_INFO **info)
>  {
> -	int err;
> +	int err = -EINVAL;
>  	struct ntfs_sb_info *sbi = ni->mi.sbi;
>  	struct ATTR_LIST_ENTRY *le = NULL;
>  	struct ATTRIB *attr_info, *attr_ea;
>  	void *ea_p;
> -	u32 size;
> +	u32 size, off, ea_size;
>  	static_assert(le32_to_cpu(ATTR_EA_INFO) < le32_to_cpu(ATTR_EA));
> @@ -96,24 +94,31 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
>  	*info = resident_data_ex(attr_info, sizeof(struct EA_INFO));
>  	if (!*info)
> -		return -EINVAL;
> +		goto out;
>  	/* Check Ea limit. */
>  	size = le32_to_cpu((*info)->size);
> -	if (size > sbi->ea_max_size)
> -		return -EFBIG;
> +	if (size > sbi->ea_max_size) {
> +		err = -EFBIG;
> +		goto out;
> +	}
> +
> +	if (attr_size(attr_ea) > sbi->ea_max_size) {
> +		err = -EFBIG;
> +		goto out;
> +	}
> -	if (attr_size(attr_ea) > sbi->ea_max_size)
> -		return -EFBIG;
> +	if (!size) {
> +		/* EA info persists, but xattr is empty. Looks like EA problem. */
> +		goto out;
> +	}
>  	/* Allocate memory for packed Ea. */
>  	ea_p = kmalloc(size_add(size, add_bytes), GFP_NOFS);
>  	if (!ea_p)
>  		return -ENOMEM;
> -	if (!size) {
> -		/* EA info persists, but xattr is empty. Looks like EA problem. */
> -	} else if (attr_ea->non_res) {
> +	if (attr_ea->non_res) {
>  		struct runs_tree run;
>  		run_init(&run);
> @@ -124,24 +129,52 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
>  		run_close(&run);
>  		if (err)
> -			goto out;
> +			goto out1;
>  	} else {
>  		void *p = resident_data_ex(attr_ea, size);
> -		if (!p) {
> -			err = -EINVAL;
> -			goto out;
> -		}
> +		if (!p)
> +			goto out1;
>  		memcpy(ea_p, p, size);
>  	}
>  	memset(Add2Ptr(ea_p, size), 0, add_bytes);
> +
> +	/* Check all attributes for consistency. */
> +	for (off = 0; off < size; off += ea_size) {
> +		const struct EA_FULL *ef = Add2Ptr(ea_p, off);
> +		u32 bytes = size - off;
> +
> +		/* Check if we can use field ea->size. */
> +		if (bytes < sizeof(ef->size))
> +			goto out1;
> +
> +		if (ef->size) {
> +			ea_size = le32_to_cpu(ef->size);
> +			if (ea_size > bytes)
> +				goto out1;
> +			continue;
> +		}
> +
> +		/* Check if we can use fields ef->name_len and ef->elength. */
> +		if (bytes < offsetof(struct EA_FULL, name))
> +			goto out1;
> +
> +		ea_size = ALIGN(struct_size(ef, name,
> +					    1 + ef->name_len +
> +						    le16_to_cpu(ef->elength)),
> +				4);
> +		if (ea_size > bytes)
> +			goto out1;
> +	}
> +
>  	*ea = ea_p;
>  	return 0;
> -out:
> +out1:
>  	kfree(ea_p);
> -	*ea = NULL;
> +out:
> +	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
>  	return err;
>  }
> @@ -163,6 +196,7 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, char *buffer,
>  	const struct EA_FULL *ea;
>  	u32 off, size;
>  	int err;
> +	int ea_size;
>  	size_t ret;
>  	err = ntfs_read_ea(ni, &ea_all, 0, &info);
> @@ -175,8 +209,9 @@ 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 < size; off += unpacked_ea_size(ea)) {
> +	for (ret = 0, off = 0; off < size; off += ea_size) {
>  		ea = Add2Ptr(ea_all, off);
> +		ea_size = unpacked_ea_size(ea);
>  		if (buffer) {
>  			if (ret + ea->name_len + 1 > bytes_per_buffer) {
> @@ -227,7 +262,8 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
>  		goto out;
>  	/* Enumerate all xattrs. */
> -	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off)) {
> +	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off,
> +		     NULL)) {
>  		err = -ENODATA;
>  		goto out;
>  	}
> @@ -269,7 +305,7 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
>  	struct EA_FULL *new_ea;
>  	struct EA_FULL *ea_all = NULL;
>  	size_t add, new_pack;
> -	u32 off, size;
> +	u32 off, size, ea_sz;
>  	__le16 size_pack;
>  	struct ATTRIB *attr;
>  	struct ATTR_LIST_ENTRY *le;
> @@ -304,9 +340,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
>  		size_pack = ea_info.size_pack;
>  	}
> -	if (info && find_ea(ea_all, size, name, name_len, &off)) {
> +	if (info && find_ea(ea_all, size, name, name_len, &off, &ea_sz)) {
>  		struct EA_FULL *ea;
> -		size_t ea_sz;
>  		if (flags & XATTR_CREATE) {
>  			err = -EEXIST;
> @@ -329,8 +364,6 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
>  		if (ea->flags & FILE_NEED_EA)
>  			le16_add_cpu(&ea_info.count, -1);
> -		ea_sz = unpacked_ea_size(ea);
> -
>  		le16_add_cpu(&ea_info.size_pack, 0 - packed_ea_size(ea));
>  		memmove(ea, Add2Ptr(ea, ea_sz), size - off - ea_sz);
> -- 
> 2.37.0
> 
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/14] fs/ntfs3: Check fields while reading
  2023-06-19  9:41   ` Lee Jones
@ 2023-06-27 13:49     ` Lee Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2023-06-27 13:49 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel

Konstantin,

On Mon, 19 Jun 2023, Lee Jones wrote:
> On Fri, 28 Oct 2022, Konstantin Komarov wrote:
> 
> > Added new functions index_hdr_check and index_buf_check.
> > Now we check all stuff for correctness while reading from disk.
> > Also fixed bug with stale nfs data.
> > 
> > Reported-by: van fantasy <g1042620637@gmail.com>
> > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> > ---
> >  fs/ntfs3/index.c   |  84 ++++++++++++++++++++++++++++++----
> >  fs/ntfs3/inode.c   |  18 ++++----
> >  fs/ntfs3/ntfs_fs.h |   4 +-
> >  fs/ntfs3/run.c     |   7 ++-
> >  fs/ntfs3/xattr.c   | 109 +++++++++++++++++++++++++++++----------------
> >  5 files changed, 164 insertions(+), 58 deletions(-)
> 
> It's not clear to me what route this patch took into Mainline [0].  My
> guess it was authored by and then merged by the maintainer after no one
> raised any review comments.
> 
> It does appear that this particular patch was identified as the fix for
> a published CVE however [1], and with no Fixes: tag I'm concerned that
> the Stable AUTOSEL bot may miss this.  With that in mind, would you mind
> submitting to the relevant Stable branches please?  [2] contains all of
> the relevant information if you're not 100% certain of the process.
> 
> Thank you.
> 
> [0] 0e8235d28f3a0 fs/ntfs3: Check fields while reading
> [1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-48502
> [2] Documentation/process/stable-kernel-rules.rst

Can you confirm receipt of this mail please?

Would you be kind enough to submit your patch to Stable please?

> > diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> > index 35369ae5c438..51ab75954640 100644
> > --- a/fs/ntfs3/index.c
> > +++ b/fs/ntfs3/index.c
> > @@ -605,11 +605,58 @@ static const struct NTFS_DE *hdr_insert_head(struct INDEX_HDR *hdr,
> >  	return e;
> >  }
> > +/*
> > + * index_hdr_check
> > + *
> > + * return true if INDEX_HDR is valid
> > + */
> > +static bool index_hdr_check(const struct INDEX_HDR *hdr, u32 bytes)
> > +{
> > +	u32 end = le32_to_cpu(hdr->used);
> > +	u32 tot = le32_to_cpu(hdr->total);
> > +	u32 off = le32_to_cpu(hdr->de_off);
> > +
> > +	if (!IS_ALIGNED(off, 8) || tot > bytes || end > tot ||
> > +	    off + sizeof(struct NTFS_DE) > end) {
> > +		/* incorrect index buffer. */
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +/*
> > + * index_buf_check
> > + *
> > + * return true if INDEX_BUFFER seems is valid
> > + */
> > +static bool index_buf_check(const struct INDEX_BUFFER *ib, u32 bytes,
> > +			    const CLST *vbn)
> > +{
> > +	const struct NTFS_RECORD_HEADER *rhdr = &ib->rhdr;
> > +	u16 fo = le16_to_cpu(rhdr->fix_off);
> > +	u16 fn = le16_to_cpu(rhdr->fix_num);
> > +
> > +	if (bytes <= offsetof(struct INDEX_BUFFER, ihdr) ||
> > +	    rhdr->sign != NTFS_INDX_SIGNATURE ||
> > +	    fo < sizeof(struct INDEX_BUFFER)
> > +	    /* Check index buffer vbn. */
> > +	    || (vbn && *vbn != le64_to_cpu(ib->vbn)) || (fo % sizeof(short)) ||
> > +	    fo + fn * sizeof(short) >= bytes ||
> > +	    fn != ((bytes >> SECTOR_SHIFT) + 1)) {
> > +		/* incorrect index buffer. */
> > +		return false;
> > +	}
> > +
> > +	return index_hdr_check(&ib->ihdr,
> > +			       bytes - offsetof(struct INDEX_BUFFER, ihdr));
> > +}
> > +
> >  void fnd_clear(struct ntfs_fnd *fnd)
> >  {
> >  	int i;
> > -	for (i = 0; i < fnd->level; i++) {
> > +	for (i = fnd->level - 1; i >= 0; i--) {
> >  		struct indx_node *n = fnd->nodes[i];
> >  		if (!n)
> > @@ -819,9 +866,16 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
> >  	u32 t32;
> >  	const struct INDEX_ROOT *root = resident_data(attr);
> > +	t32 = le32_to_cpu(attr->res.data_size);
> > +	if (t32 <= offsetof(struct INDEX_ROOT, ihdr) ||
> > +	    !index_hdr_check(&root->ihdr,
> > +			     t32 - offsetof(struct INDEX_ROOT, ihdr))) {
> > +		goto out;
> > +	}
> > +
> >  	/* Check root fields. */
> >  	if (!root->index_block_clst)
> > -		return -EINVAL;
> > +		goto out;
> >  	indx->type = type;
> >  	indx->idx2vbn_bits = __ffs(root->index_block_clst);
> > @@ -833,19 +887,19 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
> >  	if (t32 < sbi->cluster_size) {
> >  		/* Index record is smaller than a cluster, use 512 blocks. */
> >  		if (t32 != root->index_block_clst * SECTOR_SIZE)
> > -			return -EINVAL;
> > +			goto out;
> >  		/* Check alignment to a cluster. */
> >  		if ((sbi->cluster_size >> SECTOR_SHIFT) &
> >  		    (root->index_block_clst - 1)) {
> > -			return -EINVAL;
> > +			goto out;
> >  		}
> >  		indx->vbn2vbo_bits = SECTOR_SHIFT;
> >  	} else {
> >  		/* Index record must be a multiple of cluster size. */
> >  		if (t32 != root->index_block_clst << sbi->cluster_bits)
> > -			return -EINVAL;
> > +			goto out;
> >  		indx->vbn2vbo_bits = sbi->cluster_bits;
> >  	}
> > @@ -853,7 +907,14 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi,
> >  	init_rwsem(&indx->run_lock);
> >  	indx->cmp = get_cmp_func(root);
> > -	return indx->cmp ? 0 : -EINVAL;
> > +	if (!indx->cmp)
> > +		goto out;
> > +
> > +	return 0;
> > +
> > +out:
> > +	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
> > +	return -EINVAL;
> >  }
> >  static struct indx_node *indx_new(struct ntfs_index *indx,
> > @@ -1011,6 +1072,13 @@ int indx_read(struct ntfs_index *indx, struct ntfs_inode *ni, CLST vbn,
> >  		goto out;
> >  ok:
> > +	if (!index_buf_check(ib, bytes, &vbn)) {
> > +		ntfs_inode_err(&ni->vfs_inode, "directory corrupted");
> > +		ntfs_set_state(ni->mi.sbi, NTFS_DIRTY_ERROR);
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> > +
> >  	if (err == -E_NTFS_FIXUP) {
> >  		ntfs_write_bh(ni->mi.sbi, &ib->rhdr, &in->nb, 0);
> >  		err = 0;
> > @@ -1601,9 +1669,9 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni,
> >  	if (err) {
> >  		/* Restore root. */
> > -		if (mi_resize_attr(mi, attr, -ds_root))
> > +		if (mi_resize_attr(mi, attr, -ds_root)) {
> >  			memcpy(attr, a_root, asize);
> > -		else {
> > +		} else {
> >  			/* Bug? */
> >  			ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
> >  		}
> > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> > index 78ec3e6bbf67..719cf6fbb5ed 100644
> > --- a/fs/ntfs3/inode.c
> > +++ b/fs/ntfs3/inode.c
> > @@ -81,7 +81,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
> >  			 le16_to_cpu(ref->seq), le16_to_cpu(rec->seq));
> >  		goto out;
> >  	} else if (!is_rec_inuse(rec)) {
> > -		err = -EINVAL;
> > +		err = -ESTALE;
> >  		ntfs_err(sb, "Inode r=%x is not in use!", (u32)ino);
> >  		goto out;
> >  	}
> > @@ -92,8 +92,10 @@ static struct inode *ntfs_read_mft(struct inode *inode,
> >  		goto out;
> >  	}
> > -	if (!is_rec_base(rec))
> > -		goto Ok;
> > +	if (!is_rec_base(rec)) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> >  	/* Record should contain $I30 root. */
> >  	is_dir = rec->flags & RECORD_FLAG_DIR;
> > @@ -465,7 +467,6 @@ static struct inode *ntfs_read_mft(struct inode *inode,
> >  		inode->i_flags |= S_NOSEC;
> >  	}
> > -Ok:
> >  	if (ino == MFT_REC_MFT && !sb->s_root)
> >  		sbi->mft.ni = NULL;
> > @@ -519,6 +520,9 @@ struct inode *ntfs_iget5(struct super_block *sb, const struct MFT_REF *ref,
> >  		_ntfs_bad_inode(inode);
> >  	}
> > +	if (IS_ERR(inode) && name)
> > +		ntfs_set_state(sb->s_fs_info, NTFS_DIRTY_ERROR);
> > +
> >  	return inode;
> >  }
> > @@ -1652,10 +1656,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
> >  		ntfs_remove_reparse(sbi, IO_REPARSE_TAG_SYMLINK, &new_de->ref);
> >  out5:
> > -	if (S_ISDIR(mode) || run_is_empty(&ni->file.run))
> > -		goto out4;
> > -
> > -	run_deallocate(sbi, &ni->file.run, false);
> > +	if (!S_ISDIR(mode))
> > +		run_deallocate(sbi, &ni->file.run, false);
> >  out4:
> >  	clear_rec_inuse(rec);
> > diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> > index d73d1c837ba7..c9b8a6f1ba0b 100644
> > --- a/fs/ntfs3/ntfs_fs.h
> > +++ b/fs/ntfs3/ntfs_fs.h
> > @@ -795,12 +795,12 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
> >  	     u32 run_buf_size, CLST *packed_vcns);
> >  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
> >  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> > -	       u32 run_buf_size);
> > +	       int run_buf_size);
> >  #ifdef NTFS3_CHECK_FREE_CLST
> >  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
> >  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> > -		  u32 run_buf_size);
> > +		  int run_buf_size);
> >  #else
> >  #define run_unpack_ex run_unpack
> >  #endif
> > diff --git a/fs/ntfs3/run.c b/fs/ntfs3/run.c
> > index aaaa0d3d35a2..12d8682f33b5 100644
> > --- a/fs/ntfs3/run.c
> > +++ b/fs/ntfs3/run.c
> > @@ -919,12 +919,15 @@ int run_pack(const struct runs_tree *run, CLST svcn, CLST len, u8 *run_buf,
> >   */
> >  int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
> >  	       CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> > -	       u32 run_buf_size)
> > +	       int run_buf_size)
> >  {
> >  	u64 prev_lcn, vcn64, lcn, next_vcn;
> >  	const u8 *run_last, *run_0;
> >  	bool is_mft = ino == MFT_REC_MFT;
> > +	if (run_buf_size < 0)
> > +		return -EINVAL;
> > +
> >  	/* Check for empty. */
> >  	if (evcn + 1 == svcn)
> >  		return 0;
> > @@ -1046,7 +1049,7 @@ int run_unpack(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
> >   */
> >  int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
> >  		  CLST svcn, CLST evcn, CLST vcn, const u8 *run_buf,
> > -		  u32 run_buf_size)
> > +		  int run_buf_size)
> >  {
> >  	int ret, err;
> >  	CLST next_vcn, lcn, len;
> > diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> > index aeee5fb12092..385c50831a8d 100644
> > --- a/fs/ntfs3/xattr.c
> > +++ b/fs/ntfs3/xattr.c
> > @@ -42,28 +42,26 @@ static inline size_t packed_ea_size(const struct EA_FULL *ea)
> >   * Assume there is at least one xattr in the list.
> >   */
> >  static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
> > -			   const char *name, u8 name_len, u32 *off)
> > +			   const char *name, u8 name_len, u32 *off, u32 *ea_sz)
> >  {
> > -	*off = 0;
> > +	u32 ea_size;
> > -	if (!ea_all || !bytes)
> > +	*off = 0;
> > +	if (!ea_all)
> >  		return false;
> > -	for (;;) {
> > +	for (; *off < bytes; *off += ea_size) {
> >  		const struct EA_FULL *ea = Add2Ptr(ea_all, *off);
> > -		u32 next_off = *off + unpacked_ea_size(ea);
> > -
> > -		if (next_off > bytes)
> > -			return false;
> > -
> > +		ea_size = unpacked_ea_size(ea);
> >  		if (ea->name_len == name_len &&
> > -		    !memcmp(ea->name, name, name_len))
> > +		    !memcmp(ea->name, name, name_len)) {
> > +			if (ea_sz)
> > +				*ea_sz = ea_size;
> >  			return true;
> > -
> > -		*off = next_off;
> > -		if (next_off >= bytes)
> > -			return false;
> > +		}
> >  	}
> > +
> > +	return false;
> >  }
> >  /*
> > @@ -74,12 +72,12 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes,
> >  static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
> >  			size_t add_bytes, const struct EA_INFO **info)
> >  {
> > -	int err;
> > +	int err = -EINVAL;
> >  	struct ntfs_sb_info *sbi = ni->mi.sbi;
> >  	struct ATTR_LIST_ENTRY *le = NULL;
> >  	struct ATTRIB *attr_info, *attr_ea;
> >  	void *ea_p;
> > -	u32 size;
> > +	u32 size, off, ea_size;
> >  	static_assert(le32_to_cpu(ATTR_EA_INFO) < le32_to_cpu(ATTR_EA));
> > @@ -96,24 +94,31 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
> >  	*info = resident_data_ex(attr_info, sizeof(struct EA_INFO));
> >  	if (!*info)
> > -		return -EINVAL;
> > +		goto out;
> >  	/* Check Ea limit. */
> >  	size = le32_to_cpu((*info)->size);
> > -	if (size > sbi->ea_max_size)
> > -		return -EFBIG;
> > +	if (size > sbi->ea_max_size) {
> > +		err = -EFBIG;
> > +		goto out;
> > +	}
> > +
> > +	if (attr_size(attr_ea) > sbi->ea_max_size) {
> > +		err = -EFBIG;
> > +		goto out;
> > +	}
> > -	if (attr_size(attr_ea) > sbi->ea_max_size)
> > -		return -EFBIG;
> > +	if (!size) {
> > +		/* EA info persists, but xattr is empty. Looks like EA problem. */
> > +		goto out;
> > +	}
> >  	/* Allocate memory for packed Ea. */
> >  	ea_p = kmalloc(size_add(size, add_bytes), GFP_NOFS);
> >  	if (!ea_p)
> >  		return -ENOMEM;
> > -	if (!size) {
> > -		/* EA info persists, but xattr is empty. Looks like EA problem. */
> > -	} else if (attr_ea->non_res) {
> > +	if (attr_ea->non_res) {
> >  		struct runs_tree run;
> >  		run_init(&run);
> > @@ -124,24 +129,52 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
> >  		run_close(&run);
> >  		if (err)
> > -			goto out;
> > +			goto out1;
> >  	} else {
> >  		void *p = resident_data_ex(attr_ea, size);
> > -		if (!p) {
> > -			err = -EINVAL;
> > -			goto out;
> > -		}
> > +		if (!p)
> > +			goto out1;
> >  		memcpy(ea_p, p, size);
> >  	}
> >  	memset(Add2Ptr(ea_p, size), 0, add_bytes);
> > +
> > +	/* Check all attributes for consistency. */
> > +	for (off = 0; off < size; off += ea_size) {
> > +		const struct EA_FULL *ef = Add2Ptr(ea_p, off);
> > +		u32 bytes = size - off;
> > +
> > +		/* Check if we can use field ea->size. */
> > +		if (bytes < sizeof(ef->size))
> > +			goto out1;
> > +
> > +		if (ef->size) {
> > +			ea_size = le32_to_cpu(ef->size);
> > +			if (ea_size > bytes)
> > +				goto out1;
> > +			continue;
> > +		}
> > +
> > +		/* Check if we can use fields ef->name_len and ef->elength. */
> > +		if (bytes < offsetof(struct EA_FULL, name))
> > +			goto out1;
> > +
> > +		ea_size = ALIGN(struct_size(ef, name,
> > +					    1 + ef->name_len +
> > +						    le16_to_cpu(ef->elength)),
> > +				4);
> > +		if (ea_size > bytes)
> > +			goto out1;
> > +	}
> > +
> >  	*ea = ea_p;
> >  	return 0;
> > -out:
> > +out1:
> >  	kfree(ea_p);
> > -	*ea = NULL;
> > +out:
> > +	ntfs_set_state(sbi, NTFS_DIRTY_DIRTY);
> >  	return err;
> >  }
> > @@ -163,6 +196,7 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, char *buffer,
> >  	const struct EA_FULL *ea;
> >  	u32 off, size;
> >  	int err;
> > +	int ea_size;
> >  	size_t ret;
> >  	err = ntfs_read_ea(ni, &ea_all, 0, &info);
> > @@ -175,8 +209,9 @@ 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 < size; off += unpacked_ea_size(ea)) {
> > +	for (ret = 0, off = 0; off < size; off += ea_size) {
> >  		ea = Add2Ptr(ea_all, off);
> > +		ea_size = unpacked_ea_size(ea);
> >  		if (buffer) {
> >  			if (ret + ea->name_len + 1 > bytes_per_buffer) {
> > @@ -227,7 +262,8 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
> >  		goto out;
> >  	/* Enumerate all xattrs. */
> > -	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off)) {
> > +	if (!find_ea(ea_all, le32_to_cpu(info->size), name, name_len, &off,
> > +		     NULL)) {
> >  		err = -ENODATA;
> >  		goto out;
> >  	}
> > @@ -269,7 +305,7 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> >  	struct EA_FULL *new_ea;
> >  	struct EA_FULL *ea_all = NULL;
> >  	size_t add, new_pack;
> > -	u32 off, size;
> > +	u32 off, size, ea_sz;
> >  	__le16 size_pack;
> >  	struct ATTRIB *attr;
> >  	struct ATTR_LIST_ENTRY *le;
> > @@ -304,9 +340,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> >  		size_pack = ea_info.size_pack;
> >  	}
> > -	if (info && find_ea(ea_all, size, name, name_len, &off)) {
> > +	if (info && find_ea(ea_all, size, name, name_len, &off, &ea_sz)) {
> >  		struct EA_FULL *ea;
> > -		size_t ea_sz;
> >  		if (flags & XATTR_CREATE) {
> >  			err = -EEXIST;
> > @@ -329,8 +364,6 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> >  		if (ea->flags & FILE_NEED_EA)
> >  			le16_add_cpu(&ea_info.count, -1);
> > -		ea_sz = unpacked_ea_size(ea);
> > -
> >  		le16_add_cpu(&ea_info.size_pack, 0 - packed_ea_size(ea));
> >  		memmove(ea, Add2Ptr(ea, ea_sz), size - off - ea_sz);
> > -- 
> > 2.37.0
> > 
> > 
> 
> -- 
> Lee Jones [李琼斯]

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-06-27 13:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-28 17:00 [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring Konstantin Komarov
2022-10-28 17:01 ` [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters Konstantin Komarov
2022-10-28 17:02 ` [PATCH 02/14] fs/ntfs3: Change new sparse cluster processing Konstantin Komarov
2022-10-28 17:02 ` [PATCH 03/14] fs/ntfs3: Fix wrong indentations Konstantin Komarov
2022-10-28 17:03 ` [PATCH 04/14] fs/ntfs3: atomic_open implementation Konstantin Komarov
2022-10-28 17:03 ` [PATCH 05/14] fs/ntfs3: Fixing wrong logic in attr_set_size and ntfs_fallocate Konstantin Komarov
2022-10-28 17:04 ` [PATCH 06/14] fs/ntfs3: Changing locking in ntfs_rename Konstantin Komarov
2022-10-28 17:04 ` [PATCH 07/14] fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block Konstantin Komarov
2022-10-28 17:05 ` [PATCH 08/14] fs/ntfs3: Correct ntfs_check_for_free_space Konstantin Komarov
2022-10-28 17:05 ` [PATCH 09/14] fs/ntfs3: Check fields while reading Konstantin Komarov
2023-06-19  9:41   ` Lee Jones
2023-06-27 13:49     ` Lee Jones
2022-10-28 17:06 ` [PATCH 10/14] fs/ntfs3: Fix incorrect if in ntfs_set_acl_ex Konstantin Komarov
2022-10-28 17:06 ` [PATCH 11/14] fs/ntfs3: Use ALIGN kernel macro Konstantin Komarov
2022-10-28 17:07 ` [PATCH 12/14] fs/ntfs3: Fix wrong if in hdr_first_de Konstantin Komarov
2022-10-28 17:07 ` [PATCH 13/14] fs/ntfs3: Improve checking of bad clusters Konstantin Komarov
2022-10-28 17:08 ` [PATCH 14/14] fs/ntfs3: Make if more readable Konstantin Komarov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.