[PATCH] ntfs: fix undefined behavior in mft/index record size calculation

Hongling Zeng posted 1 patch 1 month ago
fs/ntfs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] ntfs: fix undefined behavior in mft/index record size calculation
Posted by Hongling Zeng 1 month ago
The boot sector validation allows clusters_per_mft_record and
clusters_per_index_record to range from 0xE1 (-31) to 0xF7 (-9) when
interpreted as signed values. When these are used as negative shift
counts in expressions like `1 << -clusters_per_mft_record`, values
like 0xE1 cause `1 << 31`, which shifts into the sign bit of a 32-bit
signed integer, resulting in undefined behavior.

Fix by using unsigned shift (1U << ...) instead of signed shift.
This prevents undefined behavior while preserving the full valid
range of negative values (-31 to -9) that may appear in NTFS boot
sectors.

The encoding scheme uses negative values to represent record sizes
smaller than cluster_size: -log2(record_size). Common values include
-10 (1024 bytes) for mft_record_size and -12 (4096 bytes) for
index_record_size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 30481e5d5dd4..a1813093222b 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -695,7 +695,7 @@ static bool parse_ntfs_boot_sector(struct ntfs_volume *vol,
 		 * = -log2(mft_record_size) bytes. mft_record_size normaly is
 		 * 1024 bytes, which is encoded as 0xF6 (-10 in decimal).
 		 */
-		vol->mft_record_size = 1 << -clusters_per_mft_record;
+		vol->mft_record_size = 1U << -clusters_per_mft_record;
 	vol->mft_record_size_mask = vol->mft_record_size - 1;
 	vol->mft_record_size_bits = ffs(vol->mft_record_size) - 1;
 	ntfs_debug("vol->mft_record_size = %i (0x%x)", vol->mft_record_size,
@@ -732,7 +732,7 @@ static bool parse_ntfs_boot_sector(struct ntfs_volume *vol,
 		 * index_record_size normaly equals 4096 bytes, which is
 		 * encoded as 0xF4 (-12 in decimal).
 		 */
-		vol->index_record_size = 1 << -clusters_per_index_record;
+		vol->index_record_size = 1U << -clusters_per_index_record;
 	vol->index_record_size_mask = vol->index_record_size - 1;
 	vol->index_record_size_bits = ffs(vol->index_record_size) - 1;
 	ntfs_debug("vol->index_record_size = %i (0x%x)",
-- 
2.25.1
Re: [PATCH] ntfs: fix undefined behavior in mft/index record size calculation
Posted by Namjae Jeon 1 month ago
On Tue, Aug 25, 2026 at 6:54 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
> The boot sector validation allows clusters_per_mft_record and
> clusters_per_index_record to range from 0xE1 (-31) to 0xF7 (-9) when
> interpreted as signed values. When these are used as negative shift
> counts in expressions like `1 << -clusters_per_mft_record`, values
> like 0xE1 cause `1 << 31`, which shifts into the sign bit of a 32-bit
> signed integer, resulting in undefined behavior.
>
> Fix by using unsigned shift (1U << ...) instead of signed shift.
> This prevents undefined behavior while preserving the full valid
> range of negative values (-31 to -9) that may appear in NTFS boot
> sectors.
>
> The encoding scheme uses negative values to represent record sizes
> smaller than cluster_size: -log2(record_size). Common values include
> -10 (1024 bytes) for mft_record_size and -12 (4096 bytes) for
> index_record_size.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Applied it to #ntfs-next with Baolin's reviewed-by tag.
Thanks!
Re: [PATCH] ntfs: fix undefined behavior in mft/index record size calculation
Posted by liubaolin 1 month ago
在 2026/8/25 17:54, Hongling Zeng 写道:
> The boot sector validation allows clusters_per_mft_record and
> clusters_per_index_record to range from 0xE1 (-31) to 0xF7 (-9) when
> interpreted as signed values. When these are used as negative shift
> counts in expressions like `1 << -clusters_per_mft_record`, values
> like 0xE1 cause `1 << 31`, which shifts into the sign bit of a 32-bit
> signed integer, resulting in undefined behavior.
> 
> Fix by using unsigned shift (1U << ...) instead of signed shift.
> This prevents undefined behavior while preserving the full valid
> range of negative values (-31 to -9) that may appear in NTFS boot
> sectors.
> 
> The encoding scheme uses negative values to represent record sizes
> smaller than cluster_size: -log2(record_size). Common values include
> -10 (1024 bytes) for mft_record_size and -12 (4096 bytes) for
> index_record_size.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/super.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 30481e5d5dd4..a1813093222b 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -695,7 +695,7 @@ static bool parse_ntfs_boot_sector(struct ntfs_volume *vol,
>   		 * = -log2(mft_record_size) bytes. mft_record_size normaly is
>   		 * 1024 bytes, which is encoded as 0xF6 (-10 in decimal).
>   		 */
> -		vol->mft_record_size = 1 << -clusters_per_mft_record;
> +		vol->mft_record_size = 1U << -clusters_per_mft_record;
>   	vol->mft_record_size_mask = vol->mft_record_size - 1;
>   	vol->mft_record_size_bits = ffs(vol->mft_record_size) - 1;
>   	ntfs_debug("vol->mft_record_size = %i (0x%x)", vol->mft_record_size,
> @@ -732,7 +732,7 @@ static bool parse_ntfs_boot_sector(struct ntfs_volume *vol,
>   		 * index_record_size normaly equals 4096 bytes, which is
>   		 * encoded as 0xF4 (-12 in decimal).
>   		 */
> -		vol->index_record_size = 1 << -clusters_per_index_record;
> +		vol->index_record_size = 1U << -clusters_per_index_record;
>   	vol->index_record_size_mask = vol->index_record_size - 1;
>   	vol->index_record_size_bits = ffs(vol->index_record_size) - 1;
>   	ntfs_debug("vol->index_record_size = %i (0x%x)",

Looks good to me.
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>