Re: [Drbd-dev] history uuids misaligned within device_statistics

[Lars Ellenberg <lars.ellenberg@linbit.com>]

On Tue, Jun 18, 2019 at 12:16:45AM -0600, David Butterfield wrote:
> I should clarify that I observed the history_uuids misalignment as a runtime error from libubsan:
> 
> drbd_nl.c:5091:21: runtime error: store to misaligned address 0x7fc223ffd33c for type 'u64', which requires 8 byte alignment
> 0x7fc223ffd33c: note: pointer points here
>   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
>               ^
> 
> 5076 static void device_to_statistics(struct device_statistics *s,
> 5077                                  struct drbd_device *device)
> ...
> 5090                 for (n = 0; n < ARRAY_SIZE(md->history_uuids); n++)
> 5091                         history_uuids[n] = md->history_uuids[n];
> 
> The history_uuids are declared with __bin_field() which does not appear to specify an alignment.
> It happens to follow a 32-bit field, so that's where it lands.
> >> 272         __bin_field(14, 0, history_uuids, HISTORY_UUIDS * sizeof(__u64))

Right, this comes out as a char[HISTORY_UUIDS * sizeof(__u64)],
and as such won't have an alignment... okay.

So maybe we should instead do memcpy?

diff --git a/drbd/drbd_nl.c b/drbd/drbd_nl.c
index adeb04e4..f77df9da 100644
--- a/drbd/drbd_nl.c
+++ b/drbd/drbd_nl.c
@@ -5074,15 +5074,13 @@ static void device_to_statistics(struct device_statistics *s,
 	s->dev_upper_blocked = !may_inc_ap_bio(device);
 	if (get_ldev(device)) {
 		struct drbd_md *md = &device->ldev->md;
-		u64 *history_uuids = (u64 *)s->history_uuids;
 		struct request_queue *q;
 		int n;
 
 		spin_lock_irq(&md->uuid_lock);
 		s->dev_current_uuid = md->current_uuid;
 		BUILD_BUG_ON(sizeof(s->history_uuids) != sizeof(md->history_uuids));
-		for (n = 0; n < ARRAY_SIZE(md->history_uuids); n++)
-			history_uuids[n] = md->history_uuids[n];
+		memcpy(s->history_uuids, md->history_uuids, sizeof(s->history_uuids));
 		s->history_uuids_len = sizeof(s->history_uuids);
 		spin_unlock_irq(&md->uuid_lock);

Or come up with a "__u64_array()" field type,
that would add an __attribute__((aligned(8)))?
 
-- 
: Lars Ellenberg
: LINBIT | Keeping the Digital World Running
: DRBD -- Heartbeat -- Corosync -- Pacemaker
: R&D, Integration, Ops, Consulting, Support

DRBD® and LINBIT® are registered trademarks of LINBIT