All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands
@ 2017-06-26 16:52 Luca Coelho
  2017-06-27 12:03 ` Arend van Spriel
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Luca Coelho @ 2017-06-26 16:52 UTC (permalink / raw)
  To: johannes
  Cc: linux-wireless, arend.vanspriel, Andrei Otcheretianski, Luca Coelho

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

If NAN interface is created with NL80211_ATTR_SOCKET_OWNER, the socket
that is used to create the interface is used for all NAN operations and
reporting NAN events.
However, it turns out that sending commands and receiving events on
the same socket is not possible in a completely race-free way:
If the socket buffer is overflowed by the events, the command response
will not be sent. In that case the caller will block forever on recv.
Using non-blocking socket for commands is more complicated and still
the command response or ack may not be received.
So, keep unicasting NAN events to the interface creator, but allow
using a different socket for commands.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

In v2:
   * Andrei fixed the documentation.

include/uapi/linux/nl80211.h | 9 ++++-----
 net/wireless/nl80211.c       | 8 --------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 828aa4703e22..51626b4175c0 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1909,11 +1909,10 @@ enum nl80211_commands {
  *	that configured the indoor setting, and the indoor operation would be
  *	cleared when the socket is closed.
  *	If set during NAN interface creation, the interface will be destroyed
- *	if the socket is closed just like any other interface. Moreover, only
- *	the netlink socket that created the interface will be allowed to add
- *	and remove functions. NAN notifications will be sent in unicast to that
- *	socket. Without this attribute, any socket can add functions and the
- *	notifications will be sent to the %NL80211_MCGRP_NAN multicast group.
+ *	if the socket is closed just like any other interface. Moreover, NAN
+ *	notifications will be sent in unicast to that socket. Without this
+ *	attribute, the notifications will be sent to the %NL80211_MCGRP_NAN
+ *	multicast group.
  *	If set during %NL80211_CMD_ASSOCIATE or %NL80211_CMD_CONNECT the
  *	station will deauthenticate when the socket is closed.
  *
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5487cd775b6f..45ba3d0872cc 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11206,10 +11206,6 @@ static int nl80211_nan_add_func(struct sk_buff *skb,
 	if (!info->attrs[NL80211_ATTR_NAN_FUNC])
 		return -EINVAL;
 
-	if (wdev->owner_nlportid &&
-	    wdev->owner_nlportid != info->snd_portid)
-		return -ENOTCONN;
-
 	err = nla_parse_nested(tb, NL80211_NAN_FUNC_ATTR_MAX,
 			       info->attrs[NL80211_ATTR_NAN_FUNC],
 			       nl80211_nan_func_policy, info->extack);
@@ -11441,10 +11437,6 @@ static int nl80211_nan_del_func(struct sk_buff *skb,
 	if (!info->attrs[NL80211_ATTR_COOKIE])
 		return -EINVAL;
 
-	if (wdev->owner_nlportid &&
-	    wdev->owner_nlportid != info->snd_portid)
-		return -ENOTCONN;
-
 	cookie = nla_get_u64(info->attrs[NL80211_ATTR_COOKIE]);
 
 	rdev_del_nan_func(rdev, wdev, cookie);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands
  2017-06-26 16:52 [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands Luca Coelho
@ 2017-06-27 12:03 ` Arend van Spriel
  2017-06-27 12:12   ` Otcheretianski, Andrei
  2017-06-29 13:24 ` Johannes Berg
  2017-06-30  6:47 ` [v2] " Kalle Valo
  2 siblings, 1 reply; 5+ messages in thread
From: Arend van Spriel @ 2017-06-27 12:03 UTC (permalink / raw)
  To: Luca Coelho, johannes; +Cc: linux-wireless, Andrei Otcheretianski, Luca Coelho

On 26-06-17 18:52, Luca Coelho wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> If NAN interface is created with NL80211_ATTR_SOCKET_OWNER, the socket
> that is used to create the interface is used for all NAN operations and
> reporting NAN events.
> However, it turns out that sending commands and receiving events on
> the same socket is not possible in a completely race-free way:
> If the socket buffer is overflowed by the events, the command response
> will not be sent. In that case the caller will block forever on recv.
> Using non-blocking socket for commands is more complicated and still
> the command response or ack may not be received.
> So, keep unicasting NAN events to the interface creator, but allow
> using a different socket for commands.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
> 
> In v2:
>    * Andrei fixed the documentation.

Almost there :-p Is the reference in NL80211_CMD_ADD_NAN_FUNCTION doc to
NL80211_ATTR_SOCKET_OWNER still warranted?

Regards,
Arend

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands
  2017-06-27 12:03 ` Arend van Spriel
@ 2017-06-27 12:12   ` Otcheretianski, Andrei
  0 siblings, 0 replies; 5+ messages in thread
From: Otcheretianski, Andrei @ 2017-06-27 12:12 UTC (permalink / raw)
  To: Arend van Spriel, Luca Coelho, johannes; +Cc: linux-wireless, Coelho, Luciano

PiBBbG1vc3QgdGhlcmUgOi1wIElzIHRoZSByZWZlcmVuY2UgaW4gTkw4MDIxMV9DTURfQUREX05B
Tl9GVU5DVElPTiBkb2MgdG8NCj4gTkw4MDIxMV9BVFRSX1NPQ0tFVF9PV05FUiBzdGlsbCB3YXJy
YW50ZWQ/DQoNCkkgdGhpbmsgeWVzLiBJdCBleHBsYWlucyB3aGVyZSBOQU4gZXZlbnRzIHdpbGwg
Z28sIGFuZCB0aGlzIGlzIHJlbGV2YW50IG9ubHkgd2hlbiB5b3UgYWRkIGZ1bmN0aW9ucy4NCg0K
Pg0KPiBSZWdhcmRzLA0KPiBBcmVuZA0K

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands
  2017-06-26 16:52 [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands Luca Coelho
  2017-06-27 12:03 ` Arend van Spriel
@ 2017-06-29 13:24 ` Johannes Berg
  2017-06-30  6:47 ` [v2] " Kalle Valo
  2 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2017-06-29 13:24 UTC (permalink / raw)
  To: Luca Coelho
  Cc: linux-wireless, arend.vanspriel, Andrei Otcheretianski, Luca Coelho

On Mon, 2017-06-26 at 19:52 +0300, Luca Coelho wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> If NAN interface is created with NL80211_ATTR_SOCKET_OWNER, the
> socket
> that is used to create the interface is used for all NAN operations
> and
> reporting NAN events.
> However, it turns out that sending commands and receiving events on
> the same socket is not possible in a completely race-free way:
> If the socket buffer is overflowed by the events, the command
> response
> will not be sent. In that case the caller will block forever on recv.
> Using non-blocking socket for commands is more complicated and still
> the command response or ack may not be received.
> So, keep unicasting NAN events to the interface creator, but allow
> using a different socket for commands.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com
> >
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> 

Agree with Andrei regarding the documentation.

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [v2] nl80211: Don't verify owner_nlportid on NAN commands
  2017-06-26 16:52 [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands Luca Coelho
  2017-06-27 12:03 ` Arend van Spriel
  2017-06-29 13:24 ` Johannes Berg
@ 2017-06-30  6:47 ` Kalle Valo
  2 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2017-06-30  6:47 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: johannes, linux-wireless, arend.vanspriel, Andrei Otcheretianski,
	Luca Coelho

Luciano Coelho <luca@coelho.fi> wrote:

> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> If NAN interface is created with NL80211_ATTR_SOCKET_OWNER, the socket
> that is used to create the interface is used for all NAN operations and
> reporting NAN events.
> However, it turns out that sending commands and receiving events on
> the same socket is not possible in a completely race-free way:
> If the socket buffer is overflowed by the events, the command response
> will not be sent. In that case the caller will block forever on recv.
> Using non-blocking socket for commands is more complicated and still
> the command response or ack may not be received.
> So, keep unicasting NAN events to the interface creator, but allow
> using a different socket for commands.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

Patch applied to wireless-drivers-next.git, thanks.

36a554cec119 nl80211: Don't verify owner_nlportid on NAN commands

-- 
https://patchwork.kernel.org/patch/9810107/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-06-30  6:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-26 16:52 [PATCH v2] nl80211: Don't verify owner_nlportid on NAN commands Luca Coelho
2017-06-27 12:03 ` Arend van Spriel
2017-06-27 12:12   ` Otcheretianski, Andrei
2017-06-29 13:24 ` Johannes Berg
2017-06-30  6:47 ` [v2] " Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.