[PATCH] notifyd: fix memory alignment

Uri Simchoni urisimchoni at gmail.com
Thu Jul 16 09:40:59 UTC 2015


Hi,

Attached pls find a fix to a memory alignment issue in notifyd.

I'm not familiar enough with the code to tell whether the
extra-allocation in the fix matters in terms of performance, but at
least this patch points at the issue.

Thanks,
Uri.
-------------- next part --------------
From 6794ef1c90fe5183b9ed1f8ebe34e96526c9cfdf Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Thu, 16 Jul 2015 12:29:12 +0300
Subject: [PATCH] notifyd: fix memory alignment in notifyd_trigger_parser

When the array of notification instances for a path is fetched
back to memory, it has to lie on a struct-aligned boundary -
This is important for archtitectures where alignment matters.

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/smbd/notifyd/notifyd.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/notifyd/notifyd.c b/source3/smbd/notifyd/notifyd.c
index d9fb11d..c84a5f6 100644
--- a/source3/smbd/notifyd/notifyd.c
+++ b/source3/smbd/notifyd/notifyd.c
@@ -327,21 +327,33 @@ int notifyd_recv(struct tevent_req *req)
  * Parse an entry in the notifyd_context->entries database
  */
 
-static bool notifyd_parse_entry(uint8_t *buf, size_t buflen,
+static bool notifyd_parse_entry(TALLOC_CTX *mem_ctx, uint8_t *buf,
+				size_t buflen,
 				struct notifyd_instance **instances,
 				size_t *num_instances)
 {
+	size_t count;
+
 	if ((buflen % sizeof(struct notifyd_instance)) != 0) {
 		DEBUG(1, ("%s: invalid buffer size: %u\n",
 			  __func__, (unsigned)buflen));
 		return false;
 	}
 
+	count = buflen / sizeof(struct notifyd_instance);
+
 	if (instances != NULL) {
-		*instances = (struct notifyd_instance *)buf;
+		/* get a memory-aligned array */
+		*instances =
+		    talloc_array(mem_ctx, struct notifyd_instance, count);
+		if (*instances == NULL) {
+			DEBUG(1, ("%s: out of memory\n", __func__));
+			return false;
+		}
+		memcpy(*instances, buf, buflen);
 	}
 	if (num_instances != NULL) {
-		*num_instances = buflen / sizeof(struct notifyd_instance);
+		*num_instances = count;
 	}
 	return true;
 }
@@ -391,7 +403,7 @@ static bool notifyd_apply_rec_change(
 	value = dbwrap_record_get_value(rec);
 
 	if (value.dsize != 0) {
-		if (!notifyd_parse_entry(value.dptr, value.dsize, NULL,
+		if (!notifyd_parse_entry(NULL, value.dptr, value.dsize, NULL,
 					 &num_instances)) {
 			goto fail;
 		}
@@ -711,10 +723,12 @@ static void notifyd_trigger_parser(TDB_DATA key, TDB_DATA data,
 	struct notifyd_instance *instances = NULL;
 	size_t num_instances = 0;
 	size_t i;
+	TALLOC_CTX *frame = talloc_stackframe();
 
-	if (!notifyd_parse_entry(data.dptr, data.dsize, &instances,
+	if (!notifyd_parse_entry(frame, data.dptr, data.dsize, &instances,
 				 &num_instances)) {
 		DEBUG(1, ("%s: Could not parse notifyd_entry\n", __func__));
+		TALLOC_FREE(frame);
 		return;
 	}
 
@@ -776,6 +790,8 @@ static void notifyd_trigger_parser(TDB_DATA key, TDB_DATA data,
 				  __func__, nt_errstr(status)));
 		}
 	}
+
+	TALLOC_FREE(frame);
 }
 
 /*
@@ -1157,7 +1173,7 @@ static int notifyd_add_proxy_syswatches(struct db_record *rec,
 	memcpy(path, key.dptr, key.dsize);
 	path[key.dsize] = '\0';
 
-	ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
+	ok = notifyd_parse_entry(db, value.dptr, value.dsize, &instances,
 				 &num_instances);
 	if (!ok) {
 		DEBUG(1, ("%s: Could not parse notifyd entry for %s\n",
@@ -1182,11 +1198,14 @@ static int notifyd_add_proxy_syswatches(struct db_record *rec,
 		}
 	}
 
+	TALLOC_FREE(instances);
+
 	return 0;
 }
 
 static int notifyd_db_del_syswatches(struct db_record *rec, void *private_data)
 {
+	struct db_context *db = dbwrap_record_get_db(rec);
 	TDB_DATA key = dbwrap_record_get_key(rec);
 	TDB_DATA value = dbwrap_record_get_value(rec);
 	struct notifyd_instance *instances = NULL;
@@ -1194,7 +1213,7 @@ static int notifyd_db_del_syswatches(struct db_record *rec, void *private_data)
 	size_t i;
 	bool ok;
 
-	ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
+	ok = notifyd_parse_entry(db, value.dptr, value.dsize, &instances,
 				 &num_instances);
 	if (!ok) {
 		DEBUG(1, ("%s: Could not parse notifyd entry for %.*s\n",
@@ -1204,6 +1223,8 @@ static int notifyd_db_del_syswatches(struct db_record *rec, void *private_data)
 	for (i=0; i<num_instances; i++) {
 		TALLOC_FREE(instances[i].sys_watch);
 	}
+
+	TALLOC_FREE(instances);
 	return 0;
 }
 
@@ -1435,11 +1456,12 @@ static bool notifyd_parse_db_parser(TDB_DATA key, TDB_DATA value,
 	size_t num_instances = 0;
 	size_t i;
 	bool ok;
+	TALLOC_CTX *frame = talloc_stackframe();
 
 	memcpy(path, key.dptr, key.dsize);
 	path[key.dsize] = 0;
 
-	ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
+	ok = notifyd_parse_entry(frame, value.dptr, value.dsize, &instances,
 				 &num_instances);
 	if (!ok) {
 		DEBUG(10, ("%s: Could not parse entry for path %s\n",
@@ -1452,10 +1474,12 @@ static bool notifyd_parse_db_parser(TDB_DATA key, TDB_DATA value,
 			       &instances[i].instance,
 			       state->private_data);
 		if (!ok) {
+			TALLOC_FREE(frame);
 			return false;
 		}
 	}
 
+	TALLOC_FREE(frame);
 	return true;
 }
 
-- 
1.9.1



More information about the samba-technical mailing list