[3/5] repair: remove BITS_PER_LONG cpp checks in bmap.[ch]
diff mbox series

Message ID 20200126113541.787884-4-hch@lst.de
State New
Headers show
Series
  • [1/5] xfsprogs: remove the ENABLE_GETTEXT substitution in platform_defs.h.in
Related show

Commit Message

Christoph Hellwig Jan. 26, 2020, 11:35 a.m. UTC
Add a little helper to validate the nex count so that we can use compile
time magic checks for sizeof long directly.  Also don't print the max
in case of an overflow as the value will always be the same.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 repair/bmap.c | 29 +++++++++++++++++++++--------
 repair/bmap.h | 13 -------------
 2 files changed, 21 insertions(+), 21 deletions(-)

Comments

Darrick J. Wong Jan. 26, 2020, 10:20 p.m. UTC | #1
On Sun, Jan 26, 2020 at 12:35:39PM +0100, Christoph Hellwig wrote:
> Add a little helper to validate the nex count so that we can use compile
> time magic checks for sizeof long directly.  Also don't print the max
> in case of an overflow as the value will always be the same.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  repair/bmap.c | 29 +++++++++++++++++++++--------
>  repair/bmap.h | 13 -------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/repair/bmap.c b/repair/bmap.c
> index 44e43ab4..0d717cd3 100644
> --- a/repair/bmap.c
> +++ b/repair/bmap.c
> @@ -22,6 +22,22 @@
>  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.
> + */
> +static inline bool
> +blkmap_nex_valid(
> +	xfs_extnum_t	nex)
> +{
> +	if (sizeof(long) < 64 && nex >= INT_MAX / sizeof(bmap_ext_t))

sizeof(long) < 8

Frankly I suspect the maximum array length on 32 bit platforms is far
less than 2^31 bytes...

--D

> +		return false;
> +	return true;
> +}
> +
>  blkmap_t *
>  blkmap_alloc(
>  	xfs_extnum_t	nex,
> @@ -35,8 +51,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 (!blkmap_nex_valid(nex)) {
>  		do_warn(
>  	_("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
>  	  "If this is not a corruption, then you will need a 64 bit system\n"
> @@ -44,7 +59,6 @@ blkmap_alloc(
>  			nex);
>  		return NULL;
>  	}
> -#endif
>  
>  	key = whichfork ? ablkmap_key : dblkmap_key;
>  	blkmap = pthread_getspecific(key);
> @@ -278,20 +292,19 @@ 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 (!blkmap_nex_valid(new_naexts)) {
>  		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.\n"),
> +			new_naexts);
>  		return NULL;
>  	}
>  
> diff --git a/repair/bmap.h b/repair/bmap.h
> index 4b588df8..df9602b3 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.24.1
>

Patch
diff mbox series

diff --git a/repair/bmap.c b/repair/bmap.c
index 44e43ab4..0d717cd3 100644
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -22,6 +22,22 @@ 
 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.
+ */
+static inline bool
+blkmap_nex_valid(
+	xfs_extnum_t	nex)
+{
+	if (sizeof(long) < 64 && nex >= INT_MAX / sizeof(bmap_ext_t))
+		return false;
+	return true;
+}
+
 blkmap_t *
 blkmap_alloc(
 	xfs_extnum_t	nex,
@@ -35,8 +51,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 (!blkmap_nex_valid(nex)) {
 		do_warn(
 	_("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
 	  "If this is not a corruption, then you will need a 64 bit system\n"
@@ -44,7 +59,6 @@  blkmap_alloc(
 			nex);
 		return NULL;
 	}
-#endif
 
 	key = whichfork ? ablkmap_key : dblkmap_key;
 	blkmap = pthread_getspecific(key);
@@ -278,20 +292,19 @@  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 (!blkmap_nex_valid(new_naexts)) {
 		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.\n"),
+			new_naexts);
 		return NULL;
 	}
 
diff --git a/repair/bmap.h b/repair/bmap.h
index 4b588df8..df9602b3 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;