diff mbox series

D-Bus: fix P2P NULL dereference after interface removal

Message ID 7745d6b39156e1d76cd08f2dd37a640aa0095908.1569432163.git.davide.caratti@gmail.com
State Superseded
Headers show
Series D-Bus: fix P2P NULL dereference after interface removal | expand

Commit Message

d. caratti Sept. 25, 2019, 5:22 p.m. UTC
when the P2P management interface is deleted, P2P is then disabled and
global->p2p_init_wpa_s is set to NULL. After that, other interfaces can
still trigger P2P functions (like wpas_p2p_find()) using d-bus. This
makes wpa_supplicant terminate with SIGSEGV, because it dereferences a
NULL pointer: fix this adding proper checks, like it's done with wpa_cli.

CC: Beniamino Galvani <bgalvani@redhat.com>
CC: Benjamin Berg <benjamin@sipsolutions.net>
Reported-by: Vladimir Benes <vbenes@redhat.com>
Signed-off-by: Davide Caratti <davide.caratti@gmail.com>
---
 wpa_supplicant/dbus/dbus_new_handlers_p2p.c | 66 ++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

Comments

Beniamino Galvani Oct. 1, 2019, 8:46 a.m. UTC | #1
On Wed, Sep 25, 2019 at 07:22:43PM +0200, Davide Caratti wrote:
> when the P2P management interface is deleted, P2P is then disabled and
> global->p2p_init_wpa_s is set to NULL. After that, other interfaces can
> still trigger P2P functions (like wpas_p2p_find()) using d-bus. This
> makes wpa_supplicant terminate with SIGSEGV, because it dereferences a
> NULL pointer: fix this adding proper checks, like it's done with wpa_cli.
> 
> CC: Beniamino Galvani <bgalvani@redhat.com>
> CC: Benjamin Berg <benjamin@sipsolutions.net>
> Reported-by: Vladimir Benes <vbenes@redhat.com>
> Signed-off-by: Davide Caratti <davide.caratti@gmail.com>
> ---
>  wpa_supplicant/dbus/dbus_new_handlers_p2p.c | 66 ++++++++++++++++++++-
>  1 file changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
> index 8cdd88564..d476cbd55 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
> @@ -40,6 +40,14 @@ static int wpas_dbus_validate_dbus_ipaddr(struct wpa_dbus_dict_entry entry)
>  }
>  
>  
> +static dbus_bool_t no_p2p_mgmt_interface(DBusError *error)
> +{
> +	dbus_set_error_const(error, WPAS_DBUS_ERROR_IFACE_UNKNOWN,
> +			     "Could not find P2P mgmt interface");
> +	return FALSE;
> +}
> +
> +
>  /**
>   * Parses out the mac address from the peer object path.
>   * @peer_path - object path of the form
> @@ -78,6 +86,22 @@ wpas_dbus_error_persistent_group_unknown(DBusMessage *message)
>  }
>  
>  
> +/**
> + * wpas_dbus_error_no_p2p_mgmt_iface - Return a new InterfaceUnknown error
> + * message
> + * @message: Pointer to incoming dbus message this error refers to
> + * Returns: a dbus error message
> + *
> + * Convenience function to create and return an unknown interface error.
> + */
> +static DBusMessage * wpas_dbus_error_no_p2p_mgmt_iface(DBusMessage *message)
> +{
> +	wpa_printf(MSG_DEBUG, "Could not find P2P mgmt interface");
> +	return dbus_message_new_error(message, WPAS_DBUS_ERROR_IFACE_UNKNOWN,
> +			              "Could not find P2P mgmt interface");
> +}
> +
> +
>  DBusMessage * wpas_dbus_handler_p2p_find(DBusMessage *message,
>  					 struct wpa_supplicant *wpa_s)
>  {
> @@ -145,6 +169,10 @@ DBusMessage * wpas_dbus_handler_p2p_find(DBusMessage *message,
>  	}
>  
>  	wpa_s = wpa_s->global->p2p_init_wpa_s;
> +	if (!wpa_s) {
> +		reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
> +		goto error;
> +	}

The reply is overwritten in the error label, so either return directly
here (freeing req_dev_types) or add a new label.

Beniamino
d. caratti Oct. 1, 2019, 9:53 a.m. UTC | #2
hello Beniamino,

On Tue, Oct 1, 2019 at 10:46 AM Beniamino Galvani <bgalvani@redhat.com> wrote:
>
> On Wed, Sep 25, 2019 at 07:22:43PM +0200, Davide Caratti wrote:
> > when the P2P management interface is deleted, P2P is then disabled and
> > global->p2p_init_wpa_s is set to NULL. After that, other interfaces can
> > still trigger P2P functions (like wpas_p2p_find()) using d-bus. This
> > makes wpa_supplicant terminate with SIGSEGV, because it dereferences a
> > NULL pointer: fix this adding proper checks, like it's done with wpa_cli.
> >
> > CC: Beniamino Galvani <bgalvani@redhat.com>
> > CC: Benjamin Berg <benjamin@sipsolutions.net>
> > Reported-by: Vladimir Benes <vbenes@redhat.com>
> > Signed-off-by: Davide Caratti <davide.caratti@gmail.com>
> > ---
> >  wpa_supplicant/dbus/dbus_new_handlers_p2p.c | 66 ++++++++++++++++++++-
> >  1 file changed, 65 insertions(+), 1 deletion(-)
> >
[...]
> > @@ -145,6 +169,10 @@ DBusMessage * wpas_dbus_handler_p2p_find(DBusMessage *message,
> >       }
> >
> >       wpa_s = wpa_s->global->p2p_init_wpa_s;
> > +     if (!wpa_s) {
> > +             reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
> > +             goto error;
> > +     }
>
> The reply is overwritten in the error label, so either return directly
> here (freeing req_dev_types) or add a new label.

thanks for spotting this, it was unintended :-)
sure, I will fix it and send a v2.
diff mbox series

Patch

diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
index 8cdd88564..d476cbd55 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
@@ -40,6 +40,14 @@  static int wpas_dbus_validate_dbus_ipaddr(struct wpa_dbus_dict_entry entry)
 }
 
 
+static dbus_bool_t no_p2p_mgmt_interface(DBusError *error)
+{
+	dbus_set_error_const(error, WPAS_DBUS_ERROR_IFACE_UNKNOWN,
+			     "Could not find P2P mgmt interface");
+	return FALSE;
+}
+
+
 /**
  * Parses out the mac address from the peer object path.
  * @peer_path - object path of the form
@@ -78,6 +86,22 @@  wpas_dbus_error_persistent_group_unknown(DBusMessage *message)
 }
 
 
+/**
+ * wpas_dbus_error_no_p2p_mgmt_iface - Return a new InterfaceUnknown error
+ * message
+ * @message: Pointer to incoming dbus message this error refers to
+ * Returns: a dbus error message
+ *
+ * Convenience function to create and return an unknown interface error.
+ */
+static DBusMessage * wpas_dbus_error_no_p2p_mgmt_iface(DBusMessage *message)
+{
+	wpa_printf(MSG_DEBUG, "Could not find P2P mgmt interface");
+	return dbus_message_new_error(message, WPAS_DBUS_ERROR_IFACE_UNKNOWN,
+			              "Could not find P2P mgmt interface");
+}
+
+
 DBusMessage * wpas_dbus_handler_p2p_find(DBusMessage *message,
 					 struct wpa_supplicant *wpa_s)
 {
@@ -145,6 +169,10 @@  DBusMessage * wpas_dbus_handler_p2p_find(DBusMessage *message,
 	}
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s) {
+		reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
+		goto error;
+	}
 
 	if (wpas_p2p_find(wpa_s, timeout, type, num_req_dev_types,
 			  req_dev_types, NULL, 0, 0, NULL, freq))
@@ -166,7 +194,9 @@  error:
 DBusMessage * wpas_dbus_handler_p2p_stop_find(DBusMessage *message,
 					      struct wpa_supplicant *wpa_s)
 {
-	wpas_p2p_stop_find(wpa_s->global->p2p_init_wpa_s);
+	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (wpa_s)
+		wpas_p2p_stop_find(wpa_s);
 	return NULL;
 }
 
@@ -185,6 +215,8 @@  DBusMessage * wpas_dbus_handler_p2p_rejectpeer(DBusMessage *message,
 		return wpas_dbus_error_invalid_args(message, NULL);
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return wpas_dbus_error_no_p2p_mgmt_iface(message);
 
 	if (wpas_p2p_reject(wpa_s, peer_addr) < 0)
 		return wpas_dbus_error_unknown_error(message,
@@ -204,6 +236,8 @@  DBusMessage * wpas_dbus_handler_p2p_listen(DBusMessage *message,
 		return wpas_dbus_error_no_memory(message);
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return wpas_dbus_error_no_p2p_mgmt_iface(message);
 
 	if (wpas_p2p_listen(wpa_s, (unsigned int) timeout)) {
 		return dbus_message_new_error(message,
@@ -245,6 +279,8 @@  DBusMessage * wpas_dbus_handler_p2p_extendedlisten(
 	}
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return wpas_dbus_error_no_p2p_mgmt_iface(message);
 
 	if (wpas_p2p_ext_listen(wpa_s, period, interval))
 		return wpas_dbus_error_unknown_error(
@@ -350,6 +386,10 @@  DBusMessage * wpas_dbus_handler_p2p_group_add(DBusMessage *message,
 	}
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s) {
+		reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
+		goto out;
+	}
 
 	if (pg_object_path != NULL) {
 		char *net_id_str;
@@ -433,6 +473,12 @@  static dbus_bool_t wpa_dbus_p2p_check_enabled(struct wpa_supplicant *wpa_s,
 				     "P2P is not available for this interface");
 		return FALSE;
 	}
+	if (!wpa_s->global->p2p_init_wpa_s) {
+		if (out_reply)
+			*out_reply = wpas_dbus_error_no_p2p_mgmt_iface(
+				message);
+		return no_p2p_mgmt_interface(error);
+	}
 	return TRUE;
 }
 
@@ -822,6 +868,8 @@  DBusMessage * wpas_dbus_handler_p2p_prov_disc_req(DBusMessage *message,
 		return wpas_dbus_error_invalid_args(message, NULL);
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return wpas_dbus_error_no_p2p_mgmt_iface(message);
 
 	if (wpas_p2p_prov_disc(wpa_s, peer_addr, config_method,
 			       WPAS_P2P_PD_FOR_GO_NEG, NULL) < 0)
@@ -1882,6 +1930,8 @@  dbus_bool_t wpas_dbus_getter_p2p_peer_groups(
 
 	wpa_s = peer_args->wpa_s;
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return no_p2p_mgmt_interface(error);
 
 	wpa_s_go = wpas_get_p2p_client_iface(wpa_s, info->p2p_device_addr);
 	if (wpa_s_go) {
@@ -1963,6 +2013,9 @@  dbus_bool_t wpas_dbus_getter_persistent_groups(
 	dbus_bool_t success = FALSE;
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return no_p2p_mgmt_interface(error);
+
 	if (!wpa_s->parent->dbus_new_path)
 		return FALSE;
 
@@ -2077,6 +2130,11 @@  DBusMessage * wpas_dbus_handler_add_persistent_group(
 	dbus_message_iter_init(message, &iter);
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s) {
+		reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
+		goto err;
+	}
+
 	if (wpa_s->parent->dbus_new_path)
 		ssid = wpa_config_add_network(wpa_s->conf);
 	if (ssid == NULL) {
@@ -2159,6 +2217,10 @@  DBusMessage * wpas_dbus_handler_remove_persistent_group(
 			      DBUS_TYPE_INVALID);
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s) {
+		reply = wpas_dbus_error_no_p2p_mgmt_iface(message);
+		goto out;
+	}
 
 	/*
 	 * Extract the network ID and ensure the network is actually a child of
@@ -2235,6 +2297,8 @@  DBusMessage * wpas_dbus_handler_remove_all_persistent_groups(
 	struct wpa_config *config;
 
 	wpa_s = wpa_s->global->p2p_init_wpa_s;
+	if (!wpa_s)
+		return wpas_dbus_error_no_p2p_mgmt_iface(message);
 
 	config = wpa_s->conf;
 	ssid = config->ssid;