core: fix semantics of nm_utils_do_sysctl()

This function gets used for both /proc/sys (ie, sysctl) and for
sysfs attributes.  There are two issues with it:

1) most sysctl values don't care about a trailing LF, but some
sysfs attributes (infiniband) do; so we always have to add the
trailing LF.  Just move that into the function to ensure that
callers don't forget to add it.

2) neither sysfs or sysctl support partial writes, while the
existing function did partial writes.  Practically, both the
write handlers for sysfs and sysctl should always handle all
the data, but if they don't, partial writes are wrong.  So
instead, try three times to write all the data.

4 files changed, 51 insertions, 32 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index 2ea3f771bf..db627091ed 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -610,27 +610,48 @@ value_hash_add_object_property (GHashTable *hash,
 gboolean
 nm_utils_do_sysctl (const char *path, const char *value)
 {
-	int fd, len, nwrote, total;
+	int fd, len, nwrote, tries;
+	char *actual;
+
+	g_return_val_if_fail (path != NULL, FALSE);
+	g_return_val_if_fail (value != NULL, FALSE);
+	g_return_val_if_fail (value[0], FALSE);
 
 	fd = open (path, O_WRONLY | O_TRUNC);
-	if (fd == -1)
+	if (fd == -1) {
+		nm_log_warn (LOGD_CORE, "sysctl: failed to open '%s': (%d) %s",
+		             path, errno, strerror (errno));
 		return FALSE;
+	}
+
+	nm_log_dbg (LOGD_CORE, "sysctl: setting '%s' to '%s'", path, value);
+
+	/* Most sysfs and sysctl options don't care about a trailing CR, while some
+	 * (like infiniband) do.  So always add the CR.  Also, neither sysfs nor
+	 * sysctl support partial writes so the CR must be added to the string we're
+	 * about to write.
+	 */
+	actual = g_strdup_printf ("%s\n", value);
 
-	len = strlen (value);
-	total = 0;
-	do {
-		nwrote = write (fd, value + total, len - total);
+	/* Try to write the entire value three times if a partial write occurs */
+	len = strlen (actual);
+	for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) {
+		nwrote = write (fd, actual, len);
 		if (nwrote == -1) {
 			if (errno == EINTR)
 				continue;
-			close (fd);
-			return FALSE;
+			break;
 		}
-		total += nwrote;
-	} while (total < len);
+	}
+	g_free (actual);
+
+	if (nwrote != len) {
+		nm_log_warn (LOGD_CORE, "sysctl: failed to set '%s' to '%s': (%d) %s",
+		             path, value, errno, strerror (errno));
+	}
 
 	close (fd);
-	return TRUE;
+	return (nwrote == len);
 }
 
 gboolean
diff --git a/src/ip6-manager/nm-ip6-manager.c b/src/ip6-manager/nm-ip6-manager.c
index 4432aa5c42..490e9574bb 100644
--- a/src/ip6-manager/nm-ip6-manager.c
+++ b/src/ip6-manager/nm-ip6-manager.c
@@ -140,7 +140,7 @@ nm_ip6_device_destroy (NMIP6Device *device)
 	/* reset the saved IPv6 value */
 	if (device->disable_ip6_save_valid) {
 		nm_utils_do_sysctl (device->disable_ip6_path,
-		                    device->disable_ip6_save ? "1\n" : "0\n");
+		                    device->disable_ip6_save ? "1" : "0");
 	}
 
 	if (device->finish_addrconf_id)
@@ -1399,10 +1399,10 @@ nm_ip6_manager_prepare_interface (NMIP6Manager *manager,
 	/* Establish target state and turn router advertisement acceptance on or off */
 	if (!strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL)) {
 		device->target_state = NM_IP6_DEVICE_GOT_LINK_LOCAL;
-		nm_utils_do_sysctl (accept_ra_path, "0\n");
+		nm_utils_do_sysctl (accept_ra_path, "0");
 	} else {
 		device->target_state = NM_IP6_DEVICE_GOT_ADDRESS;
-		nm_utils_do_sysctl (accept_ra_path, "2\n");
+		nm_utils_do_sysctl (accept_ra_path, "2");
 	}
 
 	return TRUE;
@@ -1447,9 +1447,9 @@ nm_ip6_manager_begin_addrconf (NMIP6Manager *manager, int ifindex)
 	 * new RAs; there doesn't seem to be a better way to do this right now.
 	 */
 	if (device->target_state >= NM_IP6_DEVICE_GOT_ADDRESS) {
-		nm_utils_do_sysctl (device->disable_ip6_path, "1\n");
+		nm_utils_do_sysctl (device->disable_ip6_path, "1");
 		g_usleep (200);
-		nm_utils_do_sysctl (device->disable_ip6_path, "0\n");
+		nm_utils_do_sysctl (device->disable_ip6_path, "0");
 	}
 
 	device->ip6flags_poll_id = g_timeout_add_seconds (1, poll_ip6_flags, priv->monitor);
diff --git a/src/nm-device-infiniband.c b/src/nm-device-infiniband.c
index 42187a66f8..b2915f9ee9 100644
--- a/src/nm-device-infiniband.c
+++ b/src/nm-device-infiniband.c
@@ -188,7 +188,7 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason)
 	NMConnection *connection;
 	NMSettingInfiniband *s_infiniband;
 	const char *transport_mode;
-	char *mode_path, *mode_value;
+	char *mode_path;
 	gboolean ok;
 
 	g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE);
@@ -215,9 +215,7 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason)
 		}
 	}
 
-	mode_value = g_strdup_printf ("%s\n", transport_mode);
-	ok = nm_utils_do_sysctl (mode_path, mode_value);
-	g_free (mode_value);
+	ok = nm_utils_do_sysctl (mode_path, transport_mode);
 	g_free (mode_path);
 
 	if (!ok) {
diff --git a/src/nm-device.c b/src/nm-device.c
index 64e8b728b7..27a2d2c4e2 100644
--- a/src/nm-device.c
+++ b/src/nm-device.c
@@ -2373,7 +2373,7 @@ act_stage3_ip6_config_start (NMDevice *self,
 	} else if (ip6_method_matches (connection, NM_SETTING_IP6_CONFIG_METHOD_DHCP)) {
 		/* Router advertisements shouldn't be used in pure DHCP mode */
 		if (priv->ip6_accept_ra_path)
-			nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0\n");
+			nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0");
 
 		priv->dhcp6_mode = IP6_DHCP_OPT_MANAGED;
 		ret = dhcp6_start (self, connection, priv->dhcp6_mode, reason);
@@ -2381,7 +2381,7 @@ act_stage3_ip6_config_start (NMDevice *self,
 		/* reset the saved RA value when ipv6 is ignored */
 		if (priv->ip6_accept_ra_path) {
 			nm_utils_do_sysctl (priv->ip6_accept_ra_path,
-			                    priv->ip6_accept_ra_save ? "1\n" : "0\n");
+			                    priv->ip6_accept_ra_save ? "1" : "0");
 		}
 		ret = NM_ACT_STAGE_RETURN_STOP;
 	} else if (ip6_method_matches (connection, NM_SETTING_IP6_CONFIG_METHOD_MANUAL)) {
@@ -2391,7 +2391,7 @@ act_stage3_ip6_config_start (NMDevice *self,
 
 		/* Router advertisements shouldn't be used in manual mode */
 		if (priv->ip6_accept_ra_path)
-			nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0\n");
+			nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0");
 		ret = NM_ACT_STAGE_RETURN_SUCCESS;
 	} else {
 		nm_log_warn (LOGD_IP6, "(%s): unhandled IPv6 config method; will fail",
@@ -2417,13 +2417,13 @@ act_stage3_ip6_config_start (NMDevice *self,
 	switch (ip6_privacy) {
 	case NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN:
 	case NM_SETTING_IP6_CONFIG_PRIVACY_DISABLED:
-		ip6_privacy_str = "0\n";
+		ip6_privacy_str = "0";
 	break;
 	case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_PUBLIC_ADDR:
-		ip6_privacy_str = "1\n";
+		ip6_privacy_str = "1";
 	break;
 	case NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR:
-		ip6_privacy_str = "2\n";
+		ip6_privacy_str = "2";
 	break;
 	}
 	if (priv->ip6_privacy_tempaddr_path)
@@ -2753,13 +2753,13 @@ share_init (void)
 	                    NULL };
 	char **iter;
 
-	if (!nm_utils_do_sysctl ("/proc/sys/net/ipv4/ip_forward", "1\n")) {
+	if (!nm_utils_do_sysctl ("/proc/sys/net/ipv4/ip_forward", "1")) {
 		nm_log_err (LOGD_SHARING, "Error starting IP forwarding: (%d) %s",
 					errno, strerror (errno));
 		return FALSE;
 	}
 
-	if (!nm_utils_do_sysctl ("/proc/sys/net/ipv4/ip_dynaddr", "1\n")) {
+	if (!nm_utils_do_sysctl ("/proc/sys/net/ipv4/ip_dynaddr", "1")) {
 		nm_log_err (LOGD_SHARING, "error starting IP forwarding: (%d) %s",
 					errno, strerror (errno));
 	}
@@ -3273,11 +3273,11 @@ nm_device_deactivate (NMDevice *self, NMDeviceStateReason reason)
 
 	/* Turn off router advertisements until they are needed */
 	if (priv->ip6_accept_ra_path)
-		nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0\n");
+		nm_utils_do_sysctl (priv->ip6_accept_ra_path, "0");
 
 	/* Turn off IPv6 privacy extensions */
 	if (priv->ip6_privacy_tempaddr_path)
-		nm_utils_do_sysctl (priv->ip6_privacy_tempaddr_path, "0\n");
+		nm_utils_do_sysctl (priv->ip6_privacy_tempaddr_path, "0");
 
 	/* Call device type-specific deactivation */
 	if (NM_DEVICE_GET_CLASS (self)->deactivate)
@@ -3846,7 +3846,7 @@ dispose (GObject *object)
 	/* reset the saved RA value */
 	if (priv->ip6_accept_ra_path) {
 		nm_utils_do_sysctl (priv->ip6_accept_ra_path,
-		                    priv->ip6_accept_ra_save ? "1\n" : "0\n");
+		                    priv->ip6_accept_ra_save ? "1" : "0");
 	}
 	g_free (priv->ip6_accept_ra_path);
 
@@ -3854,7 +3854,7 @@ dispose (GObject *object)
 	if (priv->ip6_privacy_tempaddr_path) {
 		char tmp[16];
 
-		snprintf (tmp, sizeof (tmp), "%d\n", priv->ip6_privacy_tempaddr_save);
+		snprintf (tmp, sizeof (tmp), "%d", priv->ip6_privacy_tempaddr_save);
 		nm_utils_do_sysctl (priv->ip6_privacy_tempaddr_path, tmp);
 	}
 	g_free (priv->ip6_privacy_tempaddr_path);