Re: [PATCH] fs/ntfs3: validate log replay bitmap buffer spans

[Konstantin Komarov <almaz.alexandrovich@paragon-software.com>]

On 8/17/26 22:29, Jérémy Jean wrote:

> do_action() derives the read buffer size for bitmap replay records from
> lrh->lcns_follow in u64, then stores it in u32 bytes. A log record that
> covers more than U32_MAX bytes therefore allocates and reads only the
> truncated tail while the bitmap range checks still compare against the
> full u64 span. ntfs_bitmap_{set,clear}_le() can then access past
> buffer_le.
>
> Keep the span in u64 until the read size has been validated against the
> u32 ntfs_read_run_nb() interface. Also validate bitmap ranges in u64 and
> reject lengths that cannot be represented by the helpers' int len
> parameter, so crafted bitmap_off/bits values cannot wrap the existing
> u32 arithmetic before the bounds check.
>
> Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> Assisted-by: Codex:gpt-5
> ---
>   fs/ntfs3/fslog.c | 53 +++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index f038c799e7ac..21d89de96ba1 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -7,6 +7,8 @@
>
>   #include <linux/blkdev.h>
>   #include <linux/fs.h>
> +#include <linux/limits.h>
> +#include <linux/overflow.h>
>   #include <linux/random.h>
>   #include <linux/slab.h>
>
> @@ -2967,6 +2969,35 @@ static inline bool check_if_alloc_index(const struct INDEX_HDR *hdr,
>          return o == attr_off;
>   }
>
> +static inline bool calc_log_buffer_size(u32 min_bytes, u64 data_bytes,
> +                                       u16 roff, bool align, u32 *bytes)
> +{
> +       u64 bytes64 = min_bytes ? min_bytes : data_bytes;
> +
> +       if (check_add_overflow(bytes64, (u64)roff, &bytes64))
> +               return false;
> +
> +       if (align) {
> +               if (check_add_overflow(bytes64, 511ULL, &bytes64))
> +                       return false;
> +               bytes64 &= ~511ULL;
> +       }
> +
> +       if (bytes64 > U32_MAX)
> +               return false;
> +
> +       *bytes = bytes64;
> +       return true;
> +}
> +
> +static inline bool check_if_bitmap_range(u64 bytes, u32 off, u32 bits)
> +{
> +       u64 start = ((u64)off + 7) / 8;
> +       u64 end = ((u64)off + bits + 7) / 8;
> +
> +       return bits <= S32_MAX && start <= bytes && end <= bytes;
> +}
> +
>   static inline void change_attr_size(struct MFT_REC *rec, struct ATTRIB *attr,
>                                      u32 nsize)
>   {
> @@ -3114,6 +3145,7 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>          u16 roff = le16_to_cpu(lrh->record_off);
>          u16 aoff = le16_to_cpu(lrh->attr_off);
>          u64 lco = 0;
> +       u64 data_bytes = 0;
>          u64 cbo = (u64)le16_to_cpu(lrh->cluster_off) << SECTOR_SHIFT;
>          u64 tvo = le64_to_cpu(lrh->target_vcn) << sbi->cluster_bits;
>          u64 vbo = cbo + tvo;
> @@ -3225,6 +3257,10 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>                  attr = oa->attr;
>                  bytes = UpdateNonresidentValue == op ? dlen : 0;
>                  lco = (u64)le16_to_cpu(lrh->lcns_follow) << sbi->cluster_bits;
> +               if (lco < cbo)
> +                       goto dirty_vol;
> +
> +               data_bytes = lco - cbo;
>
>                  if (attr->type == ATTR_ALLOC) {
>                          t32 = le32_to_cpu(oe->bytes_per_index);
> @@ -3232,12 +3268,9 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>                                  bytes = t32;
>                  }
>
> -               if (!bytes)
> -                       bytes = lco - cbo;
> -
> -               bytes += roff;
> -               if (attr->type == ATTR_ALLOC)
> -                       bytes = (bytes + 511) & ~511; // align
> +               if (!calc_log_buffer_size(bytes, data_bytes, roff,
> +                                         attr->type == ATTR_ALLOC, &bytes))
> +                       goto dirty_vol;
>
>                  buffer_le = kmalloc(bytes, GFP_NOFS);
>                  if (!buffer_le)
> @@ -3717,10 +3750,8 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>                  off = le32_to_cpu(((struct BITMAP_RANGE *)data)->bitmap_off);
>                  bits = le32_to_cpu(((struct BITMAP_RANGE *)data)->bits);
>
> -               if (cbo + (off + 7) / 8 > lco ||
> -                   cbo + ((off + bits + 7) / 8) > lco) {
> +               if (!check_if_bitmap_range(data_bytes, off, bits))
>                          goto dirty_vol;
> -               }
>
>                  ntfs_bitmap_set_le(Add2Ptr(buffer_le, roff), off, bits);
>                  a_dirty = true;
> @@ -3730,10 +3761,8 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>                  off = le32_to_cpu(((struct BITMAP_RANGE *)data)->bitmap_off);
>                  bits = le32_to_cpu(((struct BITMAP_RANGE *)data)->bits);
>
> -               if (cbo + (off + 7) / 8 > lco ||
> -                   cbo + ((off + bits + 7) / 8) > lco) {
> +               if (!check_if_bitmap_range(data_bytes, off, bits))
>                          goto dirty_vol;
> -               }
>
>                  ntfs_bitmap_clear_le(Add2Ptr(buffer_le, roff), off, bits);
>                  a_dirty = true;
> --
> 2.47.3

Hello,

Sorry for the delay.
The patch was applied, thank you.

Regards,
Konstantin