Patchwork new_dbus_handlers: Clear errno

login
register
mail settings
Submitter Paul Stewart
Date Nov. 8, 2012, 7:43 p.m.
Message ID <20121109015139.D066B200501@clearcreek.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/197908/
State Accepted
Commit 45ac5793fcbe3785282ebd190132dd59fb06c707
Headers show

Comments

Paul Stewart - Nov. 8, 2012, 7:43 p.m.
There are a few instances where dbus handlers test the value
of errno to test whether strtoul completes successfully.
Since strtoul does not clear errno, and there's no strong
reason to suspect that errno is already clear, it is safer
to clear it right before calling strtoul.  Also, any failure
in strtoul (setting errno non-zero) should be considered a
failure.

While testing using dbus-send, I found that a malformed
network path can cause a crash due to net_id being left
NULL.  We should test for this before calling strtoul
on it.

Tested with:

dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
    /fi/w1/wpa_supplicant1/Interfaces/0 \
    org.freedesktop.DBus.Properties.Get \
    string:fi.w1.wpa_supplicant1.Interface string:Networks
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0/Networks/0
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0/Networks/0
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0

Signed-hostap: Paul Stewart <pstew@chromium.org>
---
 wpa_supplicant/dbus/dbus_new_handlers.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)
Jouni Malinen - Nov. 11, 2012, 9:20 a.m.
On Thu, Nov 08, 2012 at 11:43:19AM -0800, Paul Stewart wrote:
> There are a few instances where dbus handlers test the value
> of errno to test whether strtoul completes successfully.
> Since strtoul does not clear errno, and there's no strong
> reason to suspect that errno is already clear, it is safer
> to clear it right before calling strtoul.  Also, any failure
> in strtoul (setting errno non-zero) should be considered a
> failure.
> 
> While testing using dbus-send, I found that a malformed
> network path can cause a crash due to net_id being left
> NULL.  We should test for this before calling strtoul
> on it.

Thanks, applied.

Patch

diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
index e100df2..65f7226 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers.c
@@ -1492,13 +1492,15 @@  DBusMessage * wpas_dbus_handler_remove_network(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
@@ -1592,13 +1594,15 @@  DBusMessage * wpas_dbus_handler_select_network(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
@@ -1647,13 +1651,15 @@  DBusMessage * wpas_dbus_handler_network_reply(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, net_id);
 		goto out;
 	}