[PATCH] notifyd: fix memory alignment

[Volker Lendecke]

On Fri, Jul 17, 2015 at 11:12:46AM -0700, Jeremy Allison wrote:
> On Thu, Jul 16, 2015 at 09:47:53PM +0300, Uri Simchoni wrote:
> > Best thing is to set up the DB fetching in a way that the data is
> > aligned when fetched (the parse function can verify this). I'll look
> > into it but xtdb experts can look into it without the learning curve.
> > Second best option is to have an array that grows as-needed. This
> > removes the allocations but not the copying.
> 
> Yeah, we can't avoid the memcpy I think (if you can
> think of a way I'm very interested).

What about the attached patch? Survives an initial small
test for me.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 1abb03c6a93eb120442e3f4fbda35875db8c36bd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 18 Jul 2015 21:50:55 +0200
Subject: [PATCH] dbwrap_rbt: Make "key" and "value" aligned to 16 byte

---
 lib/dbwrap/dbwrap_rbt.c |   51 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/lib/dbwrap/dbwrap_rbt.c b/lib/dbwrap/dbwrap_rbt.c
index 3f97086..03f2f57 100644
--- a/lib/dbwrap/dbwrap_rbt.c
+++ b/lib/dbwrap/dbwrap_rbt.c
@@ -38,13 +38,6 @@ struct db_rbt_rec {
 struct db_rbt_node {
 	struct rb_node rb_node;
 	size_t keysize, valuesize;
-
-	/*
-	 * key and value are appended implicitly, "data" is only here as a
-	 * target for offsetof()
-	 */
-
-	char data[1];
 };
 
 /*
@@ -83,12 +76,43 @@ static int db_rbt_compare(TDB_DATA a, TDB_DATA b)
 static void db_rbt_parse_node(struct db_rbt_node *node,
 			      TDB_DATA *key, TDB_DATA *value)
 {
-	key->dptr = ((uint8_t *)node) + offsetof(struct db_rbt_node, data);
+	size_t key_offset, value_offset;
+
+	key_offset = DBWRAP_RBT_ALIGN(sizeof(struct db_rbt_node));
+	key->dptr = ((uint8_t *)node) + key_offset;
 	key->dsize = node->keysize;
-	value->dptr = key->dptr + node->keysize;
+
+	value_offset = DBWRAP_RBT_ALIGN(node->keysize);
+	value->dptr = key->dptr + value_offset;
 	value->dsize = node->valuesize;
 }
 
+static ssize_t db_rbt_reclen(size_t keylen, size_t valuelen)
+{
+	size_t len, tmp;
+
+	len = DBWRAP_RBT_ALIGN(sizeof(struct db_rbt_node));
+
+	tmp = DBWRAP_RBT_ALIGN(keylen);
+	if (tmp < keylen) {
+		goto overflow;
+	}
+
+	len += tmp;
+	if (len < tmp) {
+		goto overflow;
+	}
+
+	len += valuelen;
+	if (len < valuelen) {
+		goto overflow;
+	}
+
+	return len;
+overflow:
+	return -1;
+}
+
 static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
 {
 	struct db_rbt_ctx *db_ctx = talloc_get_type_abort(
@@ -99,6 +123,7 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
 	struct rb_node ** p;
 	struct rb_node * parent;
 
+	ssize_t reclen;
 	TDB_DATA this_key, this_val;
 
 	if (rec_priv->node != NULL) {
@@ -123,10 +148,12 @@ static NTSTATUS db_rbt_store(struct db_record *rec, TDB_DATA data, int flag)
 		}
 	}
 
-	node = (struct db_rbt_node *)talloc_size(db_ctx,
-		offsetof(struct db_rbt_node, data) + rec->key.dsize
-		+ data.dsize);
+	reclen = db_rbt_reclen(rec->key.dsize, data.dsize);
+	if (reclen == -1) {
+		return NT_STATUS_INSUFFICIENT_RESOURCES;
+	}
 
+	node = talloc_size(db_ctx, reclen);
 	if (node == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
-- 
1.7.9.5