[PATCH v2 1/2] btrfs: validate negative error number passed to btrfs_abort_transaction()

[David Sterba <dsterba@suse.com>]

In preparation to encode more information to the error value add a step
that verifies if the value is valid (i.e. < 0). This works for
compile-time and runtime (in debugging mode).

The compile-time check recognizes direct constants and defines an array
type. An invalid condition leads to negative array size which is caught
by compiler.

The runtime check constructs the array type from the condition and only
verifies the correct size, as we don't need to tweak the size to be
negative.

The sizeof() expressions do not generate any code. In the debugging
config the warning adds about 9KiB of btrfs.ko code size.

The array size trick is needed as we can't use static_array(), not even
with __builtin_constant_p().

Sample error message:

In file included from inode.c:40:
inode.c: In function ‘__cow_file_range_inline’:
transaction.h:261:26: error: size of unnamed array is negative
  261 |         (void)sizeof(char[-!(__builtin_constant_p(error) ? (error) < 0 : 1)]);  \
      |                          ^
transaction.h:275:9: note: in expansion of macro ‘VERIFY_NEGATIVE_ERROR’
  275 |         VERIFY_NEGATIVE_ERROR(error);                           \
      |         ^~~~~~~~~~~~~~~~~~~~~
inode.c:665:17: note: in expansion of macro ‘btrfs_abort_transaction’
  665 |                 btrfs_abort_transaction(trans, 17);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index f1cb05460cec..72ab32c8ddca 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -242,6 +242,29 @@ static inline bool btrfs_abort_should_print_stack(int error)
 	return true;
 }
 
+/*
+ * Compile-time and run-time verification of error passed to transaction abort.
+ * Direct constants will be caught at compile time, errors read from variables
+ * can be caught only at run-time and will warn under debugging config.
+ *
+ * How verification works:
+ * - accepted builtin constants are all -EIO and such
+ * - for compile-time check, invalid condition produces a negative-sized array
+ *   type, valid zero-sized
+ * - when a variable is passed as error the first check is a no-op
+ * - with enabled debugging, the second array type size is constructed from the
+ *   real variable value, valid condition produces array of size 1
+ * - sizeof(type) does not generate any code
+ */
+#define VERIFY_NEGATIVE_ERROR(error)						\
+do {										\
+	(void)sizeof(char[-!(__builtin_constant_p(error) ? (error) < 0 : 1)]);	\
+	if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) {					\
+		if (sizeof(char[(error) < 0]) != 1)				\
+			DEBUG_WARN("error >= 0 passed to btrfs_abort_transaction()"); \
+	}									\
+} while(0)
+
 /*
  * Call btrfs_abort_transaction as early as possible when an error condition is
  * detected, that way the exact stack trace is reported for some errors.
@@ -249,6 +272,7 @@ static inline bool btrfs_abort_should_print_stack(int error)
 #define btrfs_abort_transaction(trans, error)		\
 do {								\
 	bool __first = false;					\
+	VERIFY_NEGATIVE_ERROR(error);				\
 	/* Report first abort since mount */			\
 	if (!test_and_set_bit(BTRFS_FS_STATE_TRANS_ABORTED,	\
 			&((trans)->fs_info->fs_state))) {	\
-- 
2.53.0