From 4665a0da497b1bd42569e5bc6da4cd9ce82f9c25 Mon Sep 17 00:00:00 2001 From: Andrey Vatoropin Date: Mon, 3 Mar 2025 09:13:56 +0000 Subject: [PATCH 1/5] fs/ntfs3: Drop redundant NULL check Static analysis shows that pointer "mi" cannot be NULL, since it is pre-initialized above. A potential failure when mi equals NULL is processed. Remove the extra NULL check. It is meaningless and harms the readability of the code, since before that the pointer is unconditionally dereferenced. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Andrey Vatoropin Signed-off-by: Konstantin Komarov --- 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 d0d530f4e2b9..e69f623b2e49 100644 --- a/fs/ntfs3/fslog.c +++ b/fs/ntfs3/fslog.c @@ -3143,7 +3143,7 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe, if (inode) iput(inode); - else if (mi) + else mi_put(mi); inode = inode_parent; From af5cab0e5b6f8edb0be51a9f47f3f620e0b4fd70 Mon Sep 17 00:00:00 2001 From: Andrey Vatoropin Date: Tue, 18 Mar 2025 13:42:18 +0000 Subject: [PATCH 2/5] fs/ntfs3: handle hdr_first_de() return value The hdr_first_de() function returns a pointer to a struct NTFS_DE. This pointer may be NULL. To handle the NULL error effectively, it is important to implement an error handler. This will help manage potential errors consistently. Additionally, error handling for the return value already exists at other points where this function is called. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") Signed-off-by: Andrey Vatoropin Signed-off-by: Konstantin Komarov --- fs/ntfs3/index.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c index 78d20e4baa2c..1bf2a6593dec 100644 --- a/fs/ntfs3/index.c +++ b/fs/ntfs3/index.c @@ -2182,6 +2182,10 @@ static int indx_get_entry_to_replace(struct ntfs_index *indx, e = hdr_first_de(&n->index->ihdr); fnd_push(fnd, n, e); + if (!e) { + err = -EINVAL; + goto out; + } if (!de_is_last(e)) { /* @@ -2203,6 +2207,10 @@ static int indx_get_entry_to_replace(struct ntfs_index *indx, n = fnd->nodes[level]; te = hdr_first_de(&n->index->ihdr); + if (!te) { + err = -EINVAL; + goto out; + } /* Copy the candidate entry into the replacement entry buffer. */ re = kmalloc(le16_to_cpu(te->size) + sizeof(u64), GFP_NOFS); if (!re) { From 8b26c8c376b29cf29710fbfd093df194cefe26ad Mon Sep 17 00:00:00 2001 From: Lizhi Xu Date: Tue, 15 Apr 2025 17:26:37 +0800 Subject: [PATCH 3/5] fs/ntfs3: Add missing direct_IO in ntfs_aops_cmpr The ntfs3 can use the page cache directly, so its address_space_operations need direct_IO. Exit ntfs_direct_IO() if it is a compressed file. Fixes: b432163ebd15 ("fs/ntfs3: Update inode->i_mapping->a_ops on compression state") Reported-by: syzbot+e36cc3297bd3afd25e19@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=e36cc3297bd3afd25e19 Signed-off-by: Lizhi Xu Signed-off-by: Konstantin Komarov --- fs/ntfs3/inode.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index 3e2957a1e360..0f0d27d4644a 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -805,6 +805,10 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) ret = 0; goto out; } + if (is_compressed(ni)) { + ret = 0; + goto out; + } ret = blockdev_direct_IO(iocb, inode, iter, wr ? ntfs_get_block_direct_IO_W : @@ -2068,5 +2072,6 @@ const struct address_space_operations ntfs_aops_cmpr = { .read_folio = ntfs_read_folio, .readahead = ntfs_readahead, .dirty_folio = block_dirty_folio, + .direct_IO = ntfs_direct_IO, }; // clang-format on From 766797c295b563449db2a3ce33342c3c04c5541a Mon Sep 17 00:00:00 2001 From: Konstantin Komarov Date: Wed, 23 Apr 2025 21:01:15 +0300 Subject: [PATCH 4/5] fs/ntfs3: Fix handling of InitializeFileRecordSegment Make the logic of handling the InitializeFileRecordSegment operation similar to that in windows. Signed-off-by: Konstantin Komarov --- fs/ntfs3/fslog.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c index e69f623b2e49..38934e6978ec 100644 --- a/fs/ntfs3/fslog.c +++ b/fs/ntfs3/fslog.c @@ -3091,16 +3091,16 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe, inode = ilookup(sbi->sb, rno); if (inode) { mi = &ntfs_i(inode)->mi; - } else if (op == InitializeFileRecordSegment) { - mi = kzalloc(sizeof(struct mft_inode), GFP_NOFS); - if (!mi) - return -ENOMEM; - err = mi_format_new(mi, sbi, rno, 0, false); - if (err) - goto out; } else { /* Read from disk. */ err = mi_get(sbi, rno, &mi); + if (err && op == InitializeFileRecordSegment) { + mi = kzalloc(sizeof(struct mft_inode), + GFP_NOFS); + if (!mi) + return -ENOMEM; + err = mi_format_new(mi, sbi, rno, 0, false); + } if (err) return err; } @@ -3109,15 +3109,13 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe, if (op == DeallocateFileRecordSegment) goto skip_load_parent; - if (InitializeFileRecordSegment != op) { - if (rec->rhdr.sign == NTFS_BAAD_SIGNATURE) - goto dirty_vol; - if (!check_lsn(&rec->rhdr, rlsn)) - goto out; - if (!check_file_record(rec, NULL, sbi)) - goto dirty_vol; - attr = Add2Ptr(rec, roff); - } + if (rec->rhdr.sign == NTFS_BAAD_SIGNATURE) + goto dirty_vol; + if (!check_lsn(&rec->rhdr, rlsn)) + goto out; + if (!check_file_record(rec, NULL, sbi)) + goto dirty_vol; + attr = Add2Ptr(rec, roff); if (is_rec_base(rec) || InitializeFileRecordSegment == op) { rno_base = rno; From eeb0819318cc0c30161821d429ca022dfbedc6ac Mon Sep 17 00:00:00 2001 From: Konstantin Komarov Date: Mon, 19 May 2025 11:07:07 +0200 Subject: [PATCH 5/5] fs/ntfs3: remove ability to change compression on mounted volume Remove all the code related to changing compression on the fly because it's not safe and not maintainable. Signed-off-by: Konstantin Komarov --- fs/ntfs3/attrib.c | 72 -------------------------------------- fs/ntfs3/file.c | 87 ---------------------------------------------- fs/ntfs3/frecord.c | 74 --------------------------------------- fs/ntfs3/namei.c | 2 -- fs/ntfs3/ntfs_fs.h | 5 --- 5 files changed, 240 deletions(-) diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c index e946f75eb540..eced9013a881 100644 --- a/fs/ntfs3/attrib.c +++ b/fs/ntfs3/attrib.c @@ -2605,75 +2605,3 @@ int attr_force_nonresident(struct ntfs_inode *ni) return err; } - -/* - * Change the compression of data attribute - */ -int attr_set_compress(struct ntfs_inode *ni, bool compr) -{ - struct ATTRIB *attr; - struct mft_inode *mi; - - attr = ni_find_attr(ni, NULL, NULL, ATTR_DATA, NULL, 0, NULL, &mi); - if (!attr) - return -ENOENT; - - if (is_attr_compressed(attr) == !!compr) { - /* Already required compressed state. */ - return 0; - } - - if (attr->non_res) { - u16 run_off; - u32 run_size; - char *run; - - if (attr->nres.data_size) { - /* - * There are rare cases when it possible to change - * compress state without big changes. - * TODO: Process these cases. - */ - return -EOPNOTSUPP; - } - - run_off = le16_to_cpu(attr->nres.run_off); - run_size = le32_to_cpu(attr->size) - run_off; - run = Add2Ptr(attr, run_off); - - if (!compr) { - /* remove field 'attr->nres.total_size'. */ - memmove(run - 8, run, run_size); - run_off -= 8; - } - - if (!mi_resize_attr(mi, attr, compr ? +8 : -8)) { - /* - * Ignore rare case when there are no 8 bytes in record with attr. - * TODO: split attribute. - */ - return -EOPNOTSUPP; - } - - if (compr) { - /* Make a gap for 'attr->nres.total_size'. */ - memmove(run + 8, run, run_size); - run_off += 8; - attr->nres.total_size = attr->nres.alloc_size; - } - attr->nres.run_off = cpu_to_le16(run_off); - } - - /* Update attribute flags. */ - if (compr) { - attr->flags &= ~ATTR_FLAG_SPARSED; - attr->flags |= ATTR_FLAG_COMPRESSED; - attr->nres.c_unit = NTFS_LZNT_CUNIT; - } else { - attr->flags &= ~ATTR_FLAG_COMPRESSED; - attr->nres.c_unit = 0; - } - mi->dirty = true; - - return 0; -} diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c index 9b6a3f8d2e7c..34ed242e1063 100644 --- a/fs/ntfs3/file.c +++ b/fs/ntfs3/file.c @@ -49,90 +49,6 @@ static int ntfs_ioctl_fitrim(struct ntfs_sb_info *sbi, unsigned long arg) return 0; } -/* - * ntfs_fileattr_get - inode_operations::fileattr_get - */ -int ntfs_fileattr_get(struct dentry *dentry, struct fileattr *fa) -{ - struct inode *inode = d_inode(dentry); - struct ntfs_inode *ni = ntfs_i(inode); - u32 flags = 0; - - if (inode->i_flags & S_IMMUTABLE) - flags |= FS_IMMUTABLE_FL; - - if (inode->i_flags & S_APPEND) - flags |= FS_APPEND_FL; - - if (is_compressed(ni)) - flags |= FS_COMPR_FL; - - if (is_encrypted(ni)) - flags |= FS_ENCRYPT_FL; - - fileattr_fill_flags(fa, flags); - - return 0; -} - -/* - * ntfs_fileattr_set - inode_operations::fileattr_set - */ -int ntfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry, - struct fileattr *fa) -{ - struct inode *inode = d_inode(dentry); - struct ntfs_inode *ni = ntfs_i(inode); - u32 flags = fa->flags; - unsigned int new_fl = 0; - - if (fileattr_has_fsx(fa)) - return -EOPNOTSUPP; - - if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_COMPR_FL)) - return -EOPNOTSUPP; - - if (flags & FS_IMMUTABLE_FL) - new_fl |= S_IMMUTABLE; - - if (flags & FS_APPEND_FL) - new_fl |= S_APPEND; - - /* Allowed to change compression for empty files and for directories only. */ - if (!is_dedup(ni) && !is_encrypted(ni) && - (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) { - int err = 0; - struct address_space *mapping = inode->i_mapping; - - /* write out all data and wait. */ - filemap_invalidate_lock(mapping); - err = filemap_write_and_wait(mapping); - - if (err >= 0) { - /* Change compress state. */ - bool compr = flags & FS_COMPR_FL; - err = ni_set_compress(inode, compr); - - /* For files change a_ops too. */ - if (!err) - mapping->a_ops = compr ? &ntfs_aops_cmpr : - &ntfs_aops; - } - - filemap_invalidate_unlock(mapping); - - if (err) - return err; - } - - inode_set_flags(inode, new_fl, S_IMMUTABLE | S_APPEND); - - inode_set_ctime_current(inode); - mark_inode_dirty(inode); - - return 0; -} - /* * ntfs_ioctl - file_operations::unlocked_ioctl */ @@ -430,7 +346,6 @@ static int ntfs_extend(struct inode *inode, loff_t pos, size_t count, } if (extend_init && !is_compressed(ni)) { - WARN_ON(ni->i_valid >= pos); err = ntfs_extend_initialized_size(file, ni, ni->i_valid, pos); if (err) goto out; @@ -1409,8 +1324,6 @@ const struct inode_operations ntfs_file_inode_operations = { .get_acl = ntfs_get_acl, .set_acl = ntfs_set_acl, .fiemap = ntfs_fiemap, - .fileattr_get = ntfs_fileattr_get, - .fileattr_set = ntfs_fileattr_set, }; const struct file_operations ntfs_file_operations = { diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c index b7a83200f2cc..756e1306fe6c 100644 --- a/fs/ntfs3/frecord.c +++ b/fs/ntfs3/frecord.c @@ -3327,77 +3327,3 @@ out: return 0; } - -/* - * ni_set_compress - * - * Helper for 'ntfs_fileattr_set'. - * Changes compression for empty files and directories only. - */ -int ni_set_compress(struct inode *inode, bool compr) -{ - int err; - struct ntfs_inode *ni = ntfs_i(inode); - struct ATTR_STD_INFO *std; - const char *bad_inode; - - if (is_compressed(ni) == !!compr) - return 0; - - if (is_sparsed(ni)) { - /* sparse and compress not compatible. */ - return -EOPNOTSUPP; - } - - if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode)) { - /*Skip other inodes. (symlink,fifo,...) */ - return -EOPNOTSUPP; - } - - bad_inode = NULL; - - ni_lock(ni); - - std = ni_std(ni); - if (!std) { - bad_inode = "no std"; - goto out; - } - - if (S_ISREG(inode->i_mode)) { - err = attr_set_compress(ni, compr); - if (err) { - if (err == -ENOENT) { - /* Fix on the fly? */ - /* Each file must contain data attribute. */ - bad_inode = "no data attribute"; - } - goto out; - } - } - - ni->std_fa = std->fa; - if (compr) { - std->fa &= ~FILE_ATTRIBUTE_SPARSE_FILE; - std->fa |= FILE_ATTRIBUTE_COMPRESSED; - } else { - std->fa &= ~FILE_ATTRIBUTE_COMPRESSED; - } - - if (ni->std_fa != std->fa) { - ni->std_fa = std->fa; - ni->mi.dirty = true; - } - /* update duplicate information and directory entries in ni_write_inode.*/ - ni->ni_flags |= NI_FLAG_UPDATE_PARENT; - err = 0; - -out: - ni_unlock(ni); - if (bad_inode) { - ntfs_bad_inode(inode, bad_inode); - err = -EINVAL; - } - - return err; -} diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c index 652735a0b0c4..b807744fc6a9 100644 --- a/fs/ntfs3/namei.c +++ b/fs/ntfs3/namei.c @@ -507,8 +507,6 @@ const struct inode_operations ntfs_dir_inode_operations = { .getattr = ntfs_getattr, .listxattr = ntfs_listxattr, .fiemap = ntfs_fiemap, - .fileattr_get = ntfs_fileattr_get, - .fileattr_set = ntfs_fileattr_set, }; const struct inode_operations ntfs_special_inode_operations = { diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h index d628977e2556..36b8052660d5 100644 --- a/fs/ntfs3/ntfs_fs.h +++ b/fs/ntfs3/ntfs_fs.h @@ -454,7 +454,6 @@ 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); -int attr_set_compress(struct ntfs_inode *ni, bool compr); /* Functions from attrlist.c */ void al_destroy(struct ntfs_inode *ni); @@ -497,9 +496,6 @@ extern const struct file_operations ntfs_dir_operations; extern const struct file_operations ntfs_legacy_dir_operations; /* Globals from file.c */ -int ntfs_fileattr_get(struct dentry *dentry, struct fileattr *fa); -int ntfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry, - struct fileattr *fa); int ntfs_getattr(struct mnt_idmap *idmap, const struct path *path, struct kstat *stat, u32 request_mask, u32 flags); int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry, @@ -585,7 +581,6 @@ int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni, bool *is_bad); bool ni_is_dirty(struct inode *inode); -int ni_set_compress(struct inode *inode, bool compr); /* Globals from fslog.c */ bool check_index_header(const struct INDEX_HDR *hdr, size_t bytes);