The macro BITFIELD used a cast of a pointer to a different anonymous struct
to (struct bitfield *). This violates strict aliasing rules, because the
two structs aren't compatible types in the sense of the language
specification. With -O2, gcc 15.1 generates code that makes th pgcmp()
function fail, because the compiler doesn't initialize bf->len aka
__storage_for__bf.len to 64.
The problem will show through error messages like this:
multipathd[1974208]: is_bit_set_in_bitfield: bitfield overflow: 1 >= 0
Fix this by using a union.
The "length" argument to BITFIELD has no effect any more. Remove it.
BITFIELD will now always have space for bits_per_slot bits (usually
64 on modern 64-bit distros).
Introduce BUILD_BUG_ON and use it in ordert to make sure static bit fields
are large enough.
Fixes: 9a2f173 ("libmpathutil: change STATIC_BITFIELD to BITFIELD")
Signed-off-by: Martin Wilck <[email protected]>
---
libmpathutil/util.c | 8 +++----
libmpathutil/util.h | 47 +++++++++++++++++++++++++--------------
libmultipath/configure.c | 6 ++---
libmultipath/discovery.c | 4 +++-
libmultipath/pgpolicies.c | 2 +-
tests/util.c | 10 ++++-----
6 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 37412c6..5e45750 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -337,15 +337,15 @@ void cleanup_fclose(void *p)
fclose(p);
}
-void cleanup_bitfield(struct bitfield **p)
+void cleanup_bitfield(union bitfield **p)
{
free(*p);
}
-struct bitfield *alloc_bitfield(unsigned int maxbit)
+union bitfield *alloc_bitfield(unsigned int maxbit)
{
unsigned int n;
- struct bitfield *bf;
+ union bitfield *bf;
if (maxbit == 0) {
errno = EINVAL;
@@ -353,7 +353,7 @@ struct bitfield *alloc_bitfield(unsigned int maxbit)
}
n = (maxbit - 1) / bits_per_slot + 1;
- bf = calloc(1, sizeof(struct bitfield) + n * sizeof(bitfield_t));
+ bf = calloc(1, sizeof(union bitfield) + (n - 1) * sizeof(bitfield_t));
if (bf)
bf->len = maxbit;
return bf;
diff --git a/libmpathutil/util.h b/libmpathutil/util.h
index 3799fd4..d2a6c52 100644
--- a/libmpathutil/util.h
+++ b/libmpathutil/util.h
@@ -16,6 +16,8 @@
#define __GLIBC_PREREQ(x, y) 0
#endif
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+
size_t strchop(char *);
const char *libmp_basename(const char *filename);
@@ -86,29 +88,40 @@ typedef unsigned int bitfield_t;
#endif
#define bits_per_slot (sizeof(bitfield_t) * CHAR_BIT)
-struct bitfield {
- unsigned int len;
- bitfield_t bits[];
+union bitfield {
+ struct {
+ unsigned int len;
+ bitfield_t bits[];
+ };
+ /* for initialization in the BITFIELD macro */
+ struct {
+ unsigned int __len;
+ bitfield_t __bits[1];
+ };
};
-#define BITFIELD(name, length) \
- struct { \
- unsigned int len; \
- bitfield_t bits[((length) - 1) / bits_per_slot + 1]; \
- } __storage_for__ ## name = { \
- .len = (length), \
- .bits = { 0, }, \
+/*
+ * BITFIELD: define a static bitfield of length bits_per_slot
+ * (aka 64 on 64-bit architectures).
+ * gcc 4.9.2 (Debian Jessie) raises an error if the initializer for
+ * .__len comes first. Thus put .__bits first.
+ * Use e.g. BUILD_BUG_ON() to make sure the bitfield size is sufficient
+ * to hold the number of bits required.
+ */
+#define BITFIELD(name) \
+ union bitfield __storage_for__ ## name = { \
+ .__bits = { 0 }, \
+ .__len = (bits_per_slot), \
}; \
- struct bitfield *name = (struct bitfield *)& __storage_for__ ## name
+ union bitfield *name = & __storage_for__ ## name
-struct bitfield *alloc_bitfield(unsigned int maxbit);
+union bitfield *alloc_bitfield(unsigned int maxbit);
void log_bitfield_overflow__(const char *f, unsigned int bit, unsigned int
len);
#define log_bitfield_overflow(bit, len) \
log_bitfield_overflow__(__func__, bit, len)
-static inline bool is_bit_set_in_bitfield(unsigned int bit,
- const struct bitfield *bf)
+static inline bool is_bit_set_in_bitfield(unsigned int bit, const union
bitfield *bf)
{
if (bit >= bf->len) {
log_bitfield_overflow(bit, bf->len);
@@ -118,7 +131,7 @@ static inline bool is_bit_set_in_bitfield(unsigned int bit,
(1ULL << (bit % bits_per_slot)));
}
-static inline void set_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
+static inline void set_bit_in_bitfield(unsigned int bit, union bitfield *bf)
{
if (bit >= bf->len) {
log_bitfield_overflow(bit, bf->len);
@@ -127,7 +140,7 @@ static inline void set_bit_in_bitfield(unsigned int bit,
struct bitfield *bf)
bf->bits[bit / bits_per_slot] |= (1ULL << (bit % bits_per_slot));
}
-static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
+static inline void clear_bit_in_bitfield(unsigned int bit, union bitfield *bf)
{
if (bit >= bf->len) {
log_bitfield_overflow(bit, bf->len);
@@ -146,5 +159,5 @@ static inline void clear_bit_in_bitfield(unsigned int bit,
struct bitfield *bf)
void cleanup_charp(char **p);
void cleanup_ucharp(unsigned char **p);
void cleanup_udev_device(struct udev_device **udd);
-void cleanup_bitfield(struct bitfield **p);
+void cleanup_bitfield(union bitfield **p);
#endif /* UTIL_H_INCLUDED */
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index a8e18a2..b5f1bab 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -439,8 +439,8 @@ static int pgcmp(struct multipath *mpp, struct multipath
*cmpp)
{
int i, j;
struct pathgroup *pgp, *cpgp;
- BITFIELD(bf, bits_per_slot);
- struct bitfield *bf__ __attribute__((cleanup(cleanup_bitfield))) =
NULL;
+ BITFIELD(bf);
+ union bitfield *bf__ __attribute__((cleanup(cleanup_bitfield))) = NULL;
if (VECTOR_SIZE(mpp->pg) != VECTOR_SIZE(cmpp->pg))
return 1;
@@ -1049,7 +1049,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec,
char *refwwid,
vector newmp;
struct config *conf = NULL;
int allow_queueing;
- struct bitfield *size_mismatch_seen;
+ union bitfield *size_mismatch_seen;
struct multipath * cmpp;
/* ignore refwwid if it's empty */
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 85ea1aa..72e64da 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -880,10 +880,12 @@ sysfs_set_nexus_loss_tmo(struct path *pp)
static void
scsi_tmo_error_msg(struct path *pp)
{
- static BITFIELD(bf, LAST_BUS_PROTOCOL_ID + 1);
+ static BITFIELD(bf);
STRBUF_ON_STACK(proto_buf);
unsigned int proto_id = bus_protocol_id(pp);
+ /* make sure the bitfield is large enough */
+ BUILD_BUG_ON((LAST_BUS_PROTOCOL_ID + 1) > bits_per_slot);
snprint_path_protocol(&proto_buf, pp);
condlog(2, "%s: setting scsi timeouts is unsupported for protocol %s",
pp->dev, get_strbuf_str(&proto_buf));
diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 2e1a543..57b7485 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -201,7 +201,7 @@ int group_by_match(struct multipath * mp, vector paths,
bool (*path_match_fn)(struct path *, struct path *))
{
int i, j;
- struct bitfield *bitmap;
+ union bitfield *bitmap;
struct path * pp;
struct pathgroup * pgp;
struct path * pp2;
diff --git a/tests/util.c b/tests/util.c
index da192ba..67d8148 100644
--- a/tests/util.c
+++ b/tests/util.c
@@ -198,7 +198,7 @@ static uint64_t maybe_swap(uint64_t v)
static void test_bitmask_1(void **state)
{
- struct bitfield *bf;
+ union bitfield *bf;
uint64_t *arr;
int i, j, k, m, b;
@@ -240,7 +240,7 @@ static void test_bitmask_1(void **state)
static void test_bitmask_2(void **state)
{
- struct bitfield *bf;
+ union bitfield *bf;
uint64_t *arr;
int i, j, k, m, b;
@@ -307,7 +307,7 @@ static void test_bitmask_2(void **state)
*/
static void test_bitmask_len_0(void **state)
{
- struct bitfield *bf;
+ union bitfield *bf;
bf = alloc_bitfield(0);
assert_null(bf);
@@ -329,7 +329,7 @@ static unsigned int maybe_swap_idx(unsigned int i)
static void _test_bitmask_small(unsigned int n)
{
- struct bitfield *bf;
+ union bitfield *bf;
uint32_t *arr;
unsigned int size = maybe_swap_idx((n - 1) / 32) + 1, i;
@@ -378,7 +378,7 @@ static void _test_bitmask_small(unsigned int n)
static void _test_bitmask_small_2(unsigned int n)
{
- struct bitfield *bf;
+ union bitfield *bf;
uint32_t *arr;
unsigned int size = maybe_swap_idx((n - 1) / 32) + 1, i;
--
2.52.0
