2022-10-28 17:32:51

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 00/14] fs/ntfs3: Additional bugfix and refactoring

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



2022-10-28 17:33:13

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 09/14] fs/ntfs3: Check fields while reading

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 <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
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



2022-10-28 17:55:19

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 03/14] fs/ntfs3: Fix wrong indentations

Also simplifying code.

Signed-off-by: Konstantin Komarov <[email protected]>
---
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



2022-10-28 17:57:42

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 01/14] fs/ntfs3: Fixing work with sparse clusters

Simplify logic in ntfs_extend_initialized_size, ntfs_sparse_cluster
and ntfs_fallocate.

Signed-off-by: Konstantin Komarov <[email protected]>
---
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



2022-10-28 17:59:48

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 07/14] fs/ntfs3: Restore correct state after ENOSPC in attr_data_get_block

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 <[email protected]>
---
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



2022-10-28 18:01:03

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 12/14] fs/ntfs3: Fix wrong if in hdr_first_de

We need to check used bytes instead of total.

Signed-off-by: Konstantin Komarov <[email protected]>
---
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



2023-06-19 10:02:33

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 09/14] fs/ntfs3: Check fields while reading

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 <[email protected]>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> 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 [李琼斯]

2023-06-27 14:31:38

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 09/14] fs/ntfs3: Check fields while reading

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 <[email protected]>
> > Signed-off-by: Konstantin Komarov <[email protected]>
> > ---
> > 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 [李琼斯]