On Thu, Feb 15, 2024 at 07:54:01AM +0100, Christoph Hellwig wrote:
> Check the 32-bit limits using sizeof instead of cpp ifdefs so that we
> can get rid of BITS_PER_LONG.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> repair/bmap.c | 23 +++++++++++++++--------
> repair/bmap.h | 13 -------------
> 2 files changed, 15 insertions(+), 21 deletions(-)
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
>
> diff --git a/repair/bmap.c b/repair/bmap.c
> index cd1a8b07b..7e32fff33 100644
> --- a/repair/bmap.c
> +++ b/repair/bmap.c
> @@ -22,6 +22,15 @@
> pthread_key_t dblkmap_key;
> pthread_key_t ablkmap_key;
>
> +/*
> + * For 32 bit platforms, we are limited to extent arrays of 2^31 bytes, which
> + * limits the number of extents in an inode we can check. If we don't limit the
> + * valid range, we can overflow the BLKMAP_SIZE() calculation and allocate less
> + * memory than we think we needed, and hence walk off the end of the array and
> + * corrupt memory.
> + */
> +#define BLKMAP_NEXTS32_MAX ((INT_MAX / sizeof(bmap_ext_t)) - 1)
> +
> blkmap_t *
> blkmap_alloc(
> xfs_extnum_t nex,
> @@ -35,8 +44,7 @@ blkmap_alloc(
> if (nex < 1)
> nex = 1;
>
> -#if (BITS_PER_LONG == 32) /* on 64-bit platforms this is never true */
> - if (nex > BLKMAP_NEXTS_MAX) {
> + if (sizeof(long) == 4 && nex > BLKMAP_NEXTS32_MAX) {
> do_warn(
> _("Number of extents requested in blkmap_alloc (%llu) overflows 32 bits.\n"
> "If this is not a corruption, then you will need a 64 bit system\n"
> @@ -44,7 +52,6 @@ blkmap_alloc(
> (unsigned long long)nex);
> return NULL;
> }
> -#endif
>
> key = whichfork ? ablkmap_key : dblkmap_key;
> blkmap = pthread_getspecific(key);
> @@ -278,20 +285,20 @@ blkmap_grow(
> ASSERT(pthread_getspecific(key) == blkmap);
> }
>
> -#if (BITS_PER_LONG == 32) /* on 64-bit platforms this is never true */
> - if (new_naexts > BLKMAP_NEXTS_MAX) {
> + if (sizeof(long) == 4 && new_naexts > BLKMAP_NEXTS32_MAX) {
> do_error(
> _("Number of extents requested in blkmap_grow (%d) overflows 32 bits.\n"
> "You need a 64 bit system to repair this filesystem.\n"),
> new_naexts);
> return NULL;
> }
> -#endif
> +
> if (new_naexts <= 0) {
> do_error(
> _("Number of extents requested in blkmap_grow (%d) overflowed the\n"
> - "maximum number of supported extents (%d).\n"),
> - new_naexts, BLKMAP_NEXTS_MAX);
> + "maximum number of supported extents (%ld).\n"),
> + new_naexts,
> + sizeof(long) == 4 ? BLKMAP_NEXTS32_MAX : INT_MAX);
> return NULL;
> }
>
> diff --git a/repair/bmap.h b/repair/bmap.h
> index 4b588df8c..df9602b31 100644
> --- a/repair/bmap.h
> +++ b/repair/bmap.h
> @@ -28,19 +28,6 @@ typedef struct blkmap {
> #define BLKMAP_SIZE(n) \
> (offsetof(blkmap_t, exts) + (sizeof(bmap_ext_t) * (n)))
>
> -/*
> - * For 32 bit platforms, we are limited to extent arrays of 2^31 bytes, which
> - * limits the number of extents in an inode we can check. If we don't limit the
> - * valid range, we can overflow the BLKMAP_SIZE() calculation and allocate less
> - * memory than we think we needed, and hence walk off the end of the array and
> - * corrupt memory.
> - */
> -#if BITS_PER_LONG == 32
> -#define BLKMAP_NEXTS_MAX ((INT_MAX / sizeof(bmap_ext_t)) - 1)
> -#else
> -#define BLKMAP_NEXTS_MAX INT_MAX
> -#endif
> -
> extern pthread_key_t dblkmap_key;
> extern pthread_key_t ablkmap_key;
>
> --
> 2.39.2
>
