From: Lee Jones <lee@kernel.org>
To: lee@kernel.org, almaz.alexandrovich@paragon-software.com
Cc: stable@vger.kernel.org, ntfs3@lists.linux.dev,
linux-fsdevel@vger.kernel.org,
van fantasy <g1042620637@gmail.com>
Subject: [PATCH 1/1] fs/ntfs3: Check fields while reading
Date: Mon, 17 Jul 2023 13:50:13 +0100 [thread overview]
Message-ID: <20230717125013.1246975-1-lee@kernel.org> (raw)
commit 0e8235d28f3a0e9eda9f02ff67ee566d5f42b66b upstream.
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>
Fixes: 82cae269cfa95 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Lee Jones <lee@kernel.org>
---
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 98491abf95b9d..9e9a9ffd92958 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)
@@ -828,9 +875,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);
@@ -842,19 +896,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;
}
@@ -862,7 +916,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,
@@ -1020,6 +1081,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;
@@ -1607,9 +1675,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 e52dfa5c7562c..dc937089a464a 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;
@@ -472,7 +474,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;
@@ -526,6 +527,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;
}
@@ -1641,10 +1645,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 c5c022fef4e0b..24227b2e1b2b0 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -794,12 +794,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 aaaa0d3d35a24..12d8682f33b53 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 ea582b4fe1d9d..884781e423e1b 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.41.0.455.g037347b96a-goog
next reply other threads:[~2023-07-17 12:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-17 12:50 Lee Jones [this message]
2023-07-17 13:39 ` [PATCH 1/1] fs/ntfs3: Check fields while reading Greg KH
2023-07-17 17:40 ` Lee Jones
2023-07-20 17:55 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230717125013.1246975-1-lee@kernel.org \
--to=lee@kernel.org \
--cc=almaz.alexandrovich@paragon-software.com \
--cc=g1042620637@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=ntfs3@lists.linux.dev \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.