2024-04-23 06:45:25

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH v2 0/9] Bugfix and refactoring

v1->v2:
Here is a reworked series of patches with corrected formatting and layout:
- two patches have been removed for finalization;
- proper commit messages were added;
- important patches have been moved to the beginning of the series
(refactoring at the end).

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

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

fs/ntfs3/attrib.c | 32 ++++++++++++++++++++++++++++++++
fs/ntfs3/file.c | 9 +++++++++
fs/ntfs3/fslog.c | 5 +++--
fs/ntfs3/inode.c | 46 ++++++++++++++++++++++++++--------------------
fs/ntfs3/namei.c | 31 ++++++++-----------------------
fs/ntfs3/ntfs.h | 2 +-
fs/ntfs3/ntfs_fs.h | 10 +++++-----
fs/ntfs3/record.c | 11 ++---------
fs/ntfs3/super.c | 2 --
fs/ntfs3/xattr.c | 5 ++++-
10 files changed, 90 insertions(+), 63 deletions(-)

--
2.34.1



2024-04-23 06:45:52

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 2/9] fs/ntfs3: Remove max link count info display during driver init

Removes the output of this purely informational message from the
kernel buffer:

"ntfs3: Max link count 4000"

Signed-off-by: Konstantin Komarov <[email protected]>
Cc: [email protected]
---
fs/ntfs3/super.c | 2 --
1 file changed, 2 deletions(-)

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

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


2024-04-23 06:46:09

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 3/9] fs/ntfs3: Missed le32_to_cpu conversion

NTFS data structure fields are stored in little-endian, it is necessary
to take this into account when working on big-endian architectures.

Fixes: 1b7dd28e14c47("fs/ntfs3: Correct function is_rst_area_valid")
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/fslog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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

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

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


2024-04-23 06:46:22

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 4/9] fs/ntfs3: Check 'folio' pointer for NULL

It can be NULL if bmap is called.

Fixes: 82cae269cfa95 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 05f169018c4e..502a527e51cd 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -571,13 +571,18 @@ static noinline int ntfs_get_block_vbo(struct inode *inode, u64 vbo,
clear_buffer_uptodate(bh);

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

--
2.34.1


2024-04-23 06:47:58

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 6/9] fs/ntfs3: Use variable length array instead of fixed size

Should fix smatch warning:
ntfs_set_label() error: __builtin_memcpy() 'uni->name' too small (20 vs 256)

Fixes: 4534a70b7056f ("fs/ntfs3: Add headers and misc files")
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/ntfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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

struct le_str {
--
2.34.1


2024-04-23 06:48:00

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 5/9] fs/ntfs3: Use 64 bit variable to avoid 32 bit overflow

For example, in the expression:
vbo = 2 * vbo + skip

Fixes: b46acd6a6a627 ("fs/ntfs3: Add NTFS journal")
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/fslog.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

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

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


2024-04-23 06:48:33

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 9/9] fs/ntfs3: Mark volume as dirty if xattr is broken

Mark a volume as corrupted if the name length exceeds the space
occupied by ea.

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/xattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 53e7d1fa036a..73785dece7a7 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -219,8 +219,11 @@ static ssize_t ntfs_list_ea(struct ntfs_inode *ni, char *buffer,
if (!ea->name_len)
break;

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

if (buffer) {
/* Check if we can use field ea->name */
--
2.34.1


2024-04-23 06:50:32

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 7/9] fs/ntfs3: Redesign ntfs_create_inode to return error code instead of inode

As Al Viro correctly pointed out, there is no need to return
the whole structure to check the error.
https://lore.kernel.org/ntfs3/20240322023515.GK538574@ZenIV/

Acked-by: Al Viro <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 22 +++++++++++-----------
fs/ntfs3/namei.c | 31 ++++++++-----------------------
fs/ntfs3/ntfs_fs.h | 9 ++++-----
3 files changed, 23 insertions(+), 39 deletions(-)

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

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

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

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

- return inode;
+ return err;
}

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

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

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

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

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

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

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


2024-04-23 06:50:50

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 8/9] fs/ntfs3: Always make file nonresident on fallocate call

xfstest 438 is starting to pass with this change.

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/attrib.c | 32 ++++++++++++++++++++++++++++++++
fs/ntfs3/file.c | 9 +++++++++
fs/ntfs3/ntfs_fs.h | 1 +
3 files changed, 42 insertions(+)

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

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

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

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