2024-01-29 08:07:00

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 0/5] fs/ntfs3: Bugfix


This series contains various fixes for ntfs3.

Konstantin Komarov (5):
  fs/ntfs3: Prevent generic message "attempt to access beyond end of
    device"
  fs/ntfs3: Use i_size_read and i_size_write
  fs/ntfs3: Correct function is_rst_area_valid
  fs/ntfs3: Fixed overflow check in mi_enum_attr()
  fs/ntfs3: Update inode->i_size after success write into compressed
    file

 fs/ntfs3/attrib.c  |  4 ++--
 fs/ntfs3/dir.c     |  2 +-
 fs/ntfs3/file.c    | 13 ++++++++-----
 fs/ntfs3/frecord.c | 10 +++++-----
 fs/ntfs3/fslog.c   | 14 ++++++++------
 fs/ntfs3/fsntfs.c  | 24 ++++++++++++++++++++++++
 fs/ntfs3/index.c   |  8 ++++----
 fs/ntfs3/inode.c   |  2 +-
 fs/ntfs3/ntfs_fs.h | 14 +-------------
 fs/ntfs3/record.c  |  2 +-
 10 files changed, 55 insertions(+), 38 deletions(-)

--
2.34.1



2024-01-29 08:08:14

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 1/5] fs/ntfs3: Prevent generic message "attempt to access beyond end of device"


It used in test environment.

Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/fsntfs.c  | 24 ++++++++++++++++++++++++
 fs/ntfs3/ntfs_fs.h | 14 +-------------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 321978019407..ae2ef5c11868 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -1007,6 +1007,30 @@ static inline __le32 security_hash(const void
*sd, size_t bytes)
     return cpu_to_le32(hash);
 }

+/*
+ * simple wrapper for sb_bread_unmovable.
+ */
+struct buffer_head *ntfs_bread(struct super_block *sb, sector_t block)
+{
+    struct ntfs_sb_info *sbi = sb->s_fs_info;
+    struct buffer_head *bh;
+
+    if (unlikely(block >= sbi->volume.blocks)) {
+        /* prevent generic message "attempt to access beyond end of
device" */
+        ntfs_err(sb, "try to read out of volume at offset 0x%llx",
+             (u64)block << sb->s_blocksize_bits);
+        return NULL;
+    }
+
+    bh = sb_bread_unmovable(sb, block);
+    if (bh)
+        return bh;
+
+    ntfs_err(sb, "failed to read volume at offset 0x%llx",
+         (u64)block << sb->s_blocksize_bits);
+    return NULL;
+}
+
 int ntfs_sb_read(struct super_block *sb, u64 lbo, size_t bytes, void
*buffer)
 {
     struct block_device *bdev = sb->s_bdev;
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 2b54ae94440f..81f7563428ee 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -586,6 +586,7 @@ bool check_index_header(const struct INDEX_HDR *hdr,
size_t bytes);
 int log_replay(struct ntfs_inode *ni, bool *initialized);

 /* Globals from fsntfs.c */
+struct buffer_head *ntfs_bread(struct super_block *sb, sector_t block);
 bool ntfs_fix_pre_write(struct NTFS_RECORD_HEADER *rhdr, size_t bytes);
 int ntfs_fix_post_read(struct NTFS_RECORD_HEADER *rhdr, size_t bytes,
                bool simple);
@@ -1032,19 +1033,6 @@ static inline u64 bytes_to_block(const struct
super_block *sb, u64 size)
     return (size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
 }

-static inline struct buffer_head *ntfs_bread(struct super_block *sb,
-                         sector_t block)
-{
-    struct buffer_head *bh = sb_bread_unmovable(sb, block);
-
-    if (bh)
-        return bh;
-
-    ntfs_err(sb, "failed to read volume at offset 0x%llx",
-         (u64)block << sb->s_blocksize_bits);
-    return NULL;
-}
-
 static inline struct ntfs_inode *ntfs_i(struct inode *inode)
 {
     return container_of(inode, struct ntfs_inode, vfs_inode);
--
2.34.1


2024-01-29 08:08:48

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 2/5] fs/ntfs3: Use i_size_read and i_size_write


Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/attrib.c  |  4 ++--
 fs/ntfs3/dir.c     |  2 +-
 fs/ntfs3/file.c    | 11 ++++++-----
 fs/ntfs3/frecord.c | 10 +++++-----
 fs/ntfs3/index.c   |  8 ++++----
 fs/ntfs3/inode.c   |  2 +-
 6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 646e2dad1b75..7aadf5010999 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -2084,7 +2084,7 @@ int attr_collapse_range(struct ntfs_inode *ni, u64
vbo, u64 bytes)

     /* Update inode size. */
     ni->i_valid = valid_size;
-    ni->vfs_inode.i_size = data_size;
+    i_size_write(&ni->vfs_inode, data_size);
     inode_set_bytes(&ni->vfs_inode, total_size);
     ni->ni_flags |= NI_FLAG_UPDATE_PARENT;
     mark_inode_dirty(&ni->vfs_inode);
@@ -2499,7 +2499,7 @@ int attr_insert_range(struct ntfs_inode *ni, u64
vbo, u64 bytes)
     mi_b->dirty = true;

 done:
-    ni->vfs_inode.i_size += bytes;
+    i_size_write(&ni->vfs_inode, ni->vfs_inode.i_size + bytes);
     ni->ni_flags |= NI_FLAG_UPDATE_PARENT;
     mark_inode_dirty(&ni->vfs_inode);

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index 9f6dd445eb04..effa6accf8a8 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -517,7 +517,7 @@ static int ntfs_dir_count(struct inode *dir, bool
*is_empty, size_t *dirs,
     u32 e_size, off, end;
     size_t drs = 0, fles = 0, bit = 0;
     struct indx_node *node = NULL;
-    size_t max_indx = ni->vfs_inode.i_size >> ni->dir.index_bits;
+    size_t max_indx = i_size_read(&ni->vfs_inode) >> ni->dir.index_bits;

     if (is_empty)
         *is_empty = true;
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 07ed3d946e7c..b702543a8795 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -646,7 +646,7 @@ static long ntfs_fallocate(struct file *file, int
mode, loff_t vbo, loff_t len)
             if (err)
                 goto out;
         } else if (new_size > i_size) {
-            inode->i_size = new_size;
+            i_size_write(inode, new_size);
         }
     }

@@ -696,7 +696,7 @@ int ntfs3_setattr(struct mnt_idmap *idmap, struct
dentry *dentry,
             goto out;
         }
         inode_dio_wait(inode);
-        oldsize = inode->i_size;
+        oldsize = i_size_read(inode);
         newsize = attr->ia_size;

         if (newsize <= oldsize)
@@ -708,7 +708,7 @@ int ntfs3_setattr(struct mnt_idmap *idmap, struct
dentry *dentry,
             goto out;

         ni->ni_flags |= NI_FLAG_UPDATE_PARENT;
-        inode->i_size = newsize;
+        i_size_write(inode, newsize);
     }

     setattr_copy(idmap, inode, attr);
@@ -847,7 +847,7 @@ static ssize_t ntfs_compress_write(struct kiocb
*iocb, struct iov_iter *from)
     size_t count = iov_iter_count(from);
     loff_t pos = iocb->ki_pos;
     struct inode *inode = file_inode(file);
-    loff_t i_size = inode->i_size;
+    loff_t i_size = i_size_read(inode);
     struct address_space *mapping = inode->i_mapping;
     struct ntfs_inode *ni = ntfs_i(inode);
     u64 valid = ni->i_valid;
@@ -1177,7 +1177,8 @@ static int ntfs_file_release(struct inode *inode,
struct file *file)
         down_write(&ni->file.run_lock);

         err = attr_set_size(ni, ATTR_DATA, NULL, 0, &ni->file.run,
-                    inode->i_size, &ni->i_valid, false, NULL);
+                    i_size_read(inode), &ni->i_valid, false,
+                    NULL);

         up_write(&ni->file.run_lock);
         ni_unlock(ni);
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 2636ab7640ac..3b42938a9d3b 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -2099,7 +2099,7 @@ int ni_readpage_cmpr(struct ntfs_inode *ni, struct
page *page)
     gfp_t gfp_mask;
     struct page *pg;

-    if (vbo >= ni->vfs_inode.i_size) {
+    if (vbo >= i_size_read(&ni->vfs_inode)) {
         SetPageUptodate(page);
         err = 0;
         goto out;
@@ -2173,7 +2173,7 @@ int ni_decompress_file(struct ntfs_inode *ni)
 {
     struct ntfs_sb_info *sbi = ni->mi.sbi;
     struct inode *inode = &ni->vfs_inode;
-    loff_t i_size = inode->i_size;
+    loff_t i_size = i_size_read(inode);
     struct address_space *mapping = inode->i_mapping;
     gfp_t gfp_mask = mapping_gfp_mask(mapping);
     struct page **pages = NULL;
@@ -2457,6 +2457,7 @@ int ni_read_frame(struct ntfs_inode *ni, u64
frame_vbo, struct page **pages,
     struct ATTR_LIST_ENTRY *le = NULL;
     struct runs_tree *run = &ni->file.run;
     u64 valid_size = ni->i_valid;
+    loff_t i_size = i_size_read(&ni->vfs_inode);
     u64 vbo_disk;
     size_t unc_size;
     u32 frame_size, i, npages_disk, ondisk_size;
@@ -2548,7 +2549,7 @@ int ni_read_frame(struct ntfs_inode *ni, u64
frame_vbo, struct page **pages,
             }
         }

-        frames = (ni->vfs_inode.i_size - 1) >> frame_bits;
+        frames = (i_size - 1) >> frame_bits;

         err = attr_wof_frame_info(ni, attr, run, frame64, frames,
                       frame_bits, &ondisk_size, &vbo_data);
@@ -2556,8 +2557,7 @@ int ni_read_frame(struct ntfs_inode *ni, u64
frame_vbo, struct page **pages,
             goto out2;

         if (frame64 == frames) {
-            unc_size = 1 + ((ni->vfs_inode.i_size - 1) &
-                    (frame_size - 1));
+            unc_size = 1 + ((i_size - 1) & (frame_size - 1));
             ondisk_size = attr_size(attr) - vbo_data;
         } else {
             unc_size = frame_size;
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index cf92b2433f7a..daabaad63aaf 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1462,7 +1462,7 @@ static int indx_create_allocate(struct ntfs_index
*indx, struct ntfs_inode *ni,
         goto out2;

     if (in->name == I30_NAME) {
-        ni->vfs_inode.i_size = data_size;
+        i_size_write(&ni->vfs_inode, data_size);
         inode_set_bytes(&ni->vfs_inode, alloc_size);
     }

@@ -1544,7 +1544,7 @@ static int indx_add_allocate(struct ntfs_index
*indx, struct ntfs_inode *ni,
     }

     if (in->name == I30_NAME)
-        ni->vfs_inode.i_size = data_size;
+        i_size_write(&ni->vfs_inode, data_size);

     *vbn = bit << indx->idx2vbn_bits;

@@ -2090,7 +2090,7 @@ static int indx_shrink(struct ntfs_index *indx,
struct ntfs_inode *ni,
         return err;

     if (in->name == I30_NAME)
-        ni->vfs_inode.i_size = new_data;
+        i_size_write(&ni->vfs_inode, new_data);

     bpb = bitmap_size(bit);
     if (bpb * 8 == nbits)
@@ -2576,7 +2576,7 @@ int indx_delete_entry(struct ntfs_index *indx,
struct ntfs_inode *ni,
         err = attr_set_size(ni, ATTR_ALLOC, in->name, in->name_len,
                     &indx->alloc_run, 0, NULL, false, NULL);
         if (in->name == I30_NAME)
-            ni->vfs_inode.i_size = 0;
+            i_size_write(&ni->vfs_inode, 0);

         err = ni_remove_attr(ni, ATTR_ALLOC, in->name, in->name_len,
                      false, NULL);
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 85452a6b1d40..eb7a8c9fba01 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -985,7 +985,7 @@ int ntfs_write_end(struct file *file, struct
address_space *mapping, loff_t pos,
         }

         if (pos + err > inode->i_size) {
-            inode->i_size = pos + err;
+            i_size_write(inode, pos + err);
             dirty = true;
         }

--
2.34.1


2024-01-29 08:08:54

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 3/5] fs/ntfs3: Correct function is_rst_area_valid


Reported-by: Robert Morris <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/fslog.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index 7dbb000fc691..855519713bf7 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -465,7 +465,7 @@ static inline bool is_rst_area_valid(const struct
RESTART_HDR *rhdr)
 {
     const struct RESTART_AREA *ra;
     u16 cl, fl, ul;
-    u32 off, l_size, file_dat_bits, file_size_round;
+    u32 off, l_size, seq_bits;
     u16 ro = le16_to_cpu(rhdr->ra_off);
     u32 sys_page = le32_to_cpu(rhdr->sys_page_size);

@@ -511,13 +511,15 @@ static inline bool is_rst_area_valid(const struct
RESTART_HDR *rhdr)
     /* Make sure the sequence number bits match the log file size. */
     l_size = le64_to_cpu(ra->l_size);

-    file_dat_bits = sizeof(u64) * 8 - le32_to_cpu(ra->seq_num_bits);
-    file_size_round = 1u << (file_dat_bits + 3);
-    if (file_size_round != l_size &&
-        (file_size_round < l_size || (file_size_round / 2) > l_size)) {
-        return false;
+    seq_bits = sizeof(u64) * 8 + 3;
+    while (l_size) {
+        l_size >>= 1;
+        seq_bits -= 1;
     }

+    if (seq_bits != ra->seq_num_bits)
+        return false;
+
     /* The log page data offset and record header length must be
quad-aligned. */
     if (!IS_ALIGNED(le16_to_cpu(ra->data_off), 8) ||
         !IS_ALIGNED(le16_to_cpu(ra->rec_hdr_len), 8))
--
2.34.1


2024-01-29 08:09:31

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 4/5] fs/ntfs3: Fixed overflow check in mi_enum_attr()


Reported-by: Robert Morris <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 7b6423584eae..6aa3a9d44df1 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -279,7 +279,7 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi,
struct ATTRIB *attr)
         if (t16 > asize)
             return NULL;

-        if (t16 + le32_to_cpu(attr->res.data_size) > asize)
+        if (le32_to_cpu(attr->res.data_size) > asize - t16)
             return NULL;

         t32 = sizeof(short) * attr->name_len;
--
2.34.1


2024-01-29 08:11:29

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 5/5] fs/ntfs3: Update inode->i_size after success write into compressed file


Reported-by: Giovanni Santini <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
 fs/ntfs3/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index b702543a8795..691b0c9b95ae 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -1054,6 +1054,8 @@ static ssize_t ntfs_compress_write(struct kiocb
*iocb, struct iov_iter *from)
     iocb->ki_pos += written;
     if (iocb->ki_pos > ni->i_valid)
         ni->i_valid = iocb->ki_pos;
+    if (iocb->ki_pos > i_size)
+        i_size_write(inode, iocb->ki_pos);

     return written;
 }
--
2.34.1


2024-02-11 12:40:17

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH 0/5] fs/ntfs3: Bugfix

On 29.01.24 09:06, Konstantin Komarov wrote:
>
> This series contains various fixes for ntfs3.
>
> Konstantin Komarov (5):
>   fs/ntfs3: Prevent generic message "attempt to access beyond end of
>     device"
>   fs/ntfs3: Use i_size_read and i_size_write
>   fs/ntfs3: Correct function is_rst_area_valid
>   fs/ntfs3: Fixed overflow check in mi_enum_attr()
>   fs/ntfs3: Update inode->i_size after success write into compressed
>     file

Thx for working on these patches that recently hit mainline.

The last patch listed above afaics (I might be wrong there!) is a fix
for https://bugzilla.kernel.org/show_bug.cgi?id=218180 (~"data added to
a newly created file that uses compression only becomes visible after a
remount or a forced cache drop"). That regression from the 6.2 days
sounds somewhat concerning, hence allow me to ask: do you plan to submit
this fix for backporting to the 6.7.y and 6.6.y series? If not: do you
think it should be save to include those there?

Side not, while at it: are any of the other patches maybe also worth
backporting?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.