From 0ec47c9b81a6fb6be44ccc852fbc7def22056ef2 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 23 Sep 2015 16:03:41 +0200

Subject: [PATCH 1/7] logging: coerce negative error values to positive errno

Especially systemd, which makes use of the error argument for logging, likes

to represent errors as negative numbers. We hence must invert a negative error

code to get the real errno.

(cherry picked from commit d6370d09e6aa158d4fc7fe1e2ce4c335eed1e017)

(cherry picked from commit 972865bc4ef67557f04fba6b7a92edb70c9d3927)

---

 src/nm-logging.c | 5 ++++-

 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/nm-logging.c b/src/nm-logging.c

index fa77034..b8433a2 100644

--- a/src/nm-logging.c

+++ b/src/nm-logging.c

@@ -381,8 +381,11 @@ _nm_log_impl (const char *file,

 		return;

 	/* Make sure that %m maps to the specified error */

-	if (error != 0)

+	if (error != 0) {

+		if (error < 0)

+			error = -error;

 		errno = error;

+	}

 	va_start (args, fmt);

 	msg = g_strdup_vprintf (fmt, args);

-- 

2.4.3

From 0c2078244f101653738460a9d6dd4983d30e08fe Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 22 Sep 2015 12:34:37 +0200

Subject: [PATCH 2/7] systemd/adapt: refactor creation of struct

 sd_event_source

(cherry picked from commit 41917a52c085b8699e5b37ef9b574ee265dcd0bf)

(cherry picked from commit 436ed50f4ab5ff152fe946f9ffdf3f2f822d0469)

---

 src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c | 16 ++++++++++++----

 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

index a6d8172..7984dcb 100644

--- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

+++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

@@ -37,6 +37,16 @@ struct sd_event_source {

 	sd_event_time_handler_t time_cb;

 };

+static struct sd_event_source *

+source_new (void)

+{

+	struct sd_event_source *source;

+

+	source = g_new0 (struct sd_event_source, 1);

+	source->refcount = 1;

+	return source;

+}

+

 int

 sd_event_source_set_priority (sd_event_source *s, int64_t priority)

 {

@@ -113,8 +123,7 @@ sd_event_add_io (sd_event *e, sd_event_source **s, int fd, uint32_t events, sd_e

 	if (!channel)

 		return -EINVAL;

-	source = g_new0 (struct sd_event_source, 1);

-	source->refcount = 1;

+	source = source_new ();

 	source->io_cb = callback;

 	source->user_data = userdata;

 	source->channel = channel;

@@ -158,8 +167,7 @@ sd_event_add_time(sd_event *e, sd_event_source **s, clockid_t clock, uint64_t us

 	struct sd_event_source *source;

 	uint64_t n = now (clock);

-	source = g_new0 (struct sd_event_source, 1);

-	source->refcount = 1;

+	source = source_new ();

 	source->time_cb = callback;

 	source->user_data = userdata;

 	source->usec = usec;

-- 

2.4.3

From f154c9ccef5d20a2aa765e84d9403e66510e9e2f Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 22 Sep 2015 12:35:42 +0200

Subject: [PATCH 3/7] systemd/adapt: use slice-allocator for struct

 sd_event_source

(cherry picked from commit fb0e87be394c0fdb4ccf2911bd089bbdf0b19113)

(cherry picked from commit 8c08014f475c4a60e129d4cd755ff30db5eac4bc)

---

 src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c | 4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

index 7984dcb..1f6038e 100644

--- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

+++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

@@ -42,7 +42,7 @@ source_new (void)

 {

 	struct sd_event_source *source;

-	source = g_new0 (struct sd_event_source, 1);

+	source = g_slice_new0 (struct sd_event_source);

 	source->refcount = 1;

 	return source;

 }

@@ -72,7 +72,7 @@ sd_event_source_unref (sd_event_source *s)

 			 */

 			g_io_channel_unref (s->channel);

 		}

-		g_free (s);

+		g_slice_free (struct sd_event_source, s);

 	}

 	return NULL;

 }

-- 

2.4.3

From 5d35919178a5a8df7804247da976352244e89cab Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 22 Sep 2015 12:44:14 +0200

Subject: [PATCH 4/7] systemd/adapt: fix potential crash invoking

 sd_event_source callbacks

It is common that the callbacks unref the event source. Hence we must

ensure that the @source stays alive until the callback returns.

(cherry picked from commit 9901047ae3464999ab5c52ac408f603c73239597)

(cherry picked from commit 55e46923c073fc5a38fcf572da05d55c0fc0b71f)

---

 src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c | 28 ++++++++++++++++++++--------

 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

index 1f6038e..67ddc8d 100644

--- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

+++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

@@ -91,6 +91,7 @@ static gboolean

 io_ready (GIOChannel *channel, GIOCondition condition, struct sd_event_source *source)

 {

 	int r, revents = 0;

+	gboolean result;

 	if (condition & G_IO_IN)

 		revents |= EPOLLIN;

@@ -103,13 +104,18 @@ io_ready (GIOChannel *channel, GIOCondition condition, struct sd_event_source *s

 	if (condition & G_IO_HUP)

 		revents |= EPOLLHUP;

+	source->refcount++;

+

 	r = source->io_cb (source, g_io_channel_unix_get_fd (channel), revents, source->user_data);

-	if (r < 0) {

+	if (r < 0 || source->refcount <= 1) {

 		source->id = 0;

-		return G_SOURCE_REMOVE;

-	}

+		result = G_SOURCE_REMOVE;

+	} else

+		result = G_SOURCE_CONTINUE;

+

+	sd_event_source_unref (source);

-	return G_SOURCE_CONTINUE;

+	return result;

 }

 int

@@ -151,14 +157,20 @@ static gboolean

 time_ready (struct sd_event_source *source)

 {

 	int r;

+	gboolean result;

+

+	source->refcount++;

 	r = source->time_cb (source, source->usec, source->user_data);

-	if (r < 0) {

+	if (r < 0 || source->refcount <= 1) {

 		source->id = 0;

-		return G_SOURCE_REMOVE;

-	}

+		result = G_SOURCE_REMOVE;

+	} else

+		result = G_SOURCE_CONTINUE;

+

+	sd_event_source_unref (source);

-	return G_SOURCE_CONTINUE;

+	return result;

 }

 int

-- 

2.4.3

From bfd1b1daaaed47a7c91c4561c7de29091cdf702d Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 22 Sep 2015 13:26:58 +0200

Subject: [PATCH 5/7] systemd/adapt: assert that a @source argument is passed

 to sd_event_add_time()

Systemd supports omitting the output source argument. In this case,

the created event source is floating and the reference count

is handled properly.

We have however no callers that make use of that functionality, so

instead of implementing floating references, assert that we don't

need it.

This isn't a change in behavior, because previously the could would just

SEGFAULT if a caller didn't want to take ownership of the created event.

(cherry picked from commit 02c51d4231da154b624fe5859a1e34f340eecb05)

(cherry picked from commit 11237a0a66df5e2486d961050aa03103578c40d2)

---

 src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c | 8 ++++++++

 1 file changed, 8 insertions(+)

diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

index 67ddc8d..1f8f2dc 100644

--- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

+++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

@@ -125,6 +125,10 @@ sd_event_add_io (sd_event *e, sd_event_source **s, int fd, uint32_t events, sd_e

 	GIOChannel *channel;

 	GIOCondition condition = 0;

+	/* systemd supports floating sd_event_source by omitting the @s argument.

+	 * We don't have such users and don't implement floating references. */

+	g_return_val_if_fail (s, -EINVAL);

+

 	channel = g_io_channel_unix_new (fd);

 	if (!channel)

 		return -EINVAL;

@@ -179,6 +183,10 @@ sd_event_add_time(sd_event *e, sd_event_source **s, clockid_t clock, uint64_t us

 	struct sd_event_source *source;

 	uint64_t n = now (clock);

+	/* systemd supports floating sd_event_source by omitting the @s argument.

+	 * We don't have such users and don't implement floating references. */

+	g_return_val_if_fail (s, -EINVAL);

+

 	source = source_new ();

 	source->time_cb = callback;

 	source->user_data = userdata;

-- 

2.4.3

From 04d5235aa7e7cff5ec6e62e73a8a8d47bd478ab7 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 23 Sep 2015 09:48:00 +0200

Subject: [PATCH 6/7] systemd/adapt: refactor sd_event_source to use a union

 for holding mutually exclusive fields

sd_event_source is either used for sd_event_add_io() or sd_event_add_time().

Depending on the use, different fields of the struct are relevant. Refactor

the struct to have a union.

This reduces the size of the struct, but more importantly, it makes it

clear which fields are used in which context.

(cherry picked from commit 2d2d742cf1787a948a20bf7a12acd17fa62e1dc9)

(cherry picked from commit 0e6a13bccc6503657781f91960b82212af7317f8)

---

 src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c | 22 ++++++++++++++--------

 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

index 1f8f2dc..24b77c5 100644

--- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

+++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.c

@@ -31,10 +31,16 @@ struct sd_event_source {

 	gpointer user_data;

 	GIOChannel *channel;

-	sd_event_io_handler_t io_cb;

-	uint64_t usec;

-	sd_event_time_handler_t time_cb;

+	union {

+		struct {

+			sd_event_io_handler_t cb;

+		} io;

+		struct {

+			sd_event_time_handler_t cb;

+			uint64_t usec;

+		} time;

+	};

 };

 static struct sd_event_source *

@@ -106,7 +112,7 @@ io_ready (GIOChannel *channel, GIOCondition condition, struct sd_event_source *s

 	source->refcount++;

-	r = source->io_cb (source, g_io_channel_unix_get_fd (channel), revents, source->user_data);

+	r = source->io.cb (source, g_io_channel_unix_get_fd (channel), revents, source->user_data);

 	if (r < 0 || source->refcount <= 1) {

 		source->id = 0;

 		result = G_SOURCE_REMOVE;

@@ -134,7 +140,7 @@ sd_event_add_io (sd_event *e, sd_event_source **s, int fd, uint32_t events, sd_e

 		return -EINVAL;

 	source = source_new ();

-	source->io_cb = callback;

+	source->io.cb = callback;

 	source->user_data = userdata;

 	source->channel = channel;

@@ -165,7 +171,7 @@ time_ready (struct sd_event_source *source)

 	source->refcount++;

-	r = source->time_cb (source, source->usec, source->user_data);

+	r = source->time.cb (source, source->time.usec, source->user_data);

 	if (r < 0 || source->refcount <= 1) {

 		source->id = 0;

 		result = G_SOURCE_REMOVE;

@@ -188,9 +194,9 @@ sd_event_add_time(sd_event *e, sd_event_source **s, clockid_t clock, uint64_t us

 	g_return_val_if_fail (s, -EINVAL);

 	source = source_new ();

-	source->time_cb = callback;

+	source->time.cb = callback;

 	source->user_data = userdata;

-	source->usec = usec;

+	source->time.usec = usec;

 	if (usec > 1000)

 		usec = n < usec - 1000 ? usec - n : 1000;

-- 

2.4.3

From 5980ff8b1106f2adb55a9ce6665e76d85d70734d Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 22 Sep 2015 14:03:32 +0200

Subject: [PATCH 7/7] systemd: avoid potential crash due to uncanceled timers

 in client_receive_advertise()

Got a crash with unknown reason on nm-1-0 branch. It's unclear why,

but the reason could be that a lease in client_receive_advertise()

was cleared, but not its timers.

Backtrace from nm-1-0 branch (note that the systemd code where the crash

happend is different, but similar):

    #0  sd_event_source_unref (s=0xf5c007e8fb894853) at dhcp-manager/systemd-dhcp/nm-sd-adapt.c:53

    #1  0x0000555555682340 in client_timeout_t1 (s=<optimized out>, usec=<optimized out>, userdata=0x5555559f5240)

        at dhcp-manager/systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c:451

    #2  0x00005555556a078f in time_ready (source=0x5555559f3c20) at dhcp-manager/systemd-dhcp/nm-sd-adapt.c:146

    #3  0x00007ffff4a481b3 in g_timeout_dispatch () from /lib64/libglib-2.0.so.0

    #4  0x00007ffff4a4779a in g_main_context_dispatch () from /lib64/libglib-2.0.so.0

    #5  0x00007ffff4a47ae8 in g_main_context_iterate.isra.24 () from /lib64/libglib-2.0.so.0

    #6  0x00007ffff4a47dba in g_main_loop_run () from /lib64/libglib-2.0.so.0

    #7  0x0000555555597073 in main (argc=1, argv=0x7fffffffe2b8) at main.c:512

Equivalent upstream systemd patch:

  https://github.com/systemd/systemd/pull/1332

  https://github.com/systemd/systemd/commit/f89087272b5561c9a3fc9d6a4e2a09f75f688fa7

https://bugzilla.redhat.com/show_bug.cgi?id=1260727

(cherry picked from commit 401a2eb834bdddd8931161eb86204eb80d26b2c7)

(cherry picked from commit fe70a87f942fa4a5d509787852185a7168739de6)

---

 .../systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c            | 5 ++++-

 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/dhcp-manager/systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c b/src/dhcp-manager/systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c

index 6c5a6ab..8a17a72 100644

--- a/src/dhcp-manager/systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c

+++ b/src/dhcp-manager/systemd-dhcp/src/libsystemd-network/sd-dhcp6-client.c

@@ -828,7 +828,10 @@ static int client_receive_advertise(sd_dhcp6_client *client,

         r = dhcp6_lease_get_preference(client->lease, &pref_lease);

         if (!client->lease || r < 0 || pref_advertise > pref_lease) {

-                sd_dhcp6_lease_unref(client->lease);

+                if (client->lease) {

+                        dhcp6_lease_clear_timers(&client->lease->ia);

+                        sd_dhcp6_lease_unref(client->lease);

+                }

                 client->lease = lease;

                 lease = NULL;

                 r = 0;

-- 

2.4.3