| Message ID | 20170626165230.13971-1-luca@coelho.fi (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 36a554cec119bbd20c4ec0cb96bd4712d124bfea |
| Delegated to: | Kalle Valo |
| Headers | show |
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
> Almost there :-p Is the reference in NL80211_CMD_ADD_NAN_FUNCTION doc to > NL80211_ATTR_SOCKET_OWNER still warranted? I think yes. It explains where NAN events will go, and this is relevant only when you add functions. > > Regards, > Arend
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
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
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);