From: Arend Van Spriel <arend.vanspriel@broadcom.com>
To: Denis Kenzior <denkenz@gmail.com>,
Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2 2/3] nl80211: Limit certain commands to interface owner
Date: Fri, 21 Jun 2019 10:09:04 +0200 [thread overview]
Message-ID: <11852f40-67e5-9122-7d82-077bdd0b014a@broadcom.com> (raw)
In-Reply-To: <20190620220749.10071-2-denkenz@gmail.com>
On 6/21/2019 12:07 AM, Denis Kenzior wrote:
> If the wdev object has been created (via NEW_INTERFACE) with
> SOCKET_OWNER attribute set, then limit certain commands only to the
> process that created that wdev.
>
> This can be used to make sure no other process on the system interferes
> by sending unwanted scans, action frames or any other funny business.
The flag is a good addition opposed to having handlers deal with it.
However, earlier motivation for SOCKET_OWNER use was about netlink
multicast being unreliable, which I can agree to. However, avoiding
"funny business" is a different thing. Our testing infrastructure is
doing all kind of funny business. Guess we will need to refrain from
using any user-space wireless tools that use the SOCKET_OWNER attribute,
but how do we know? Somehow I suspect iwd is one to avoid ;-) I have yet
to give iwd a spin, but this SOCKET_OWNER strategy kept me from it.
Maybe iwd could have a developer option which disables the use of the
SOCKET_OWNER attribute.
Regards,
Arend
> This patch introduces a new internal flag, and checks that flag in the
> pre_doit hook.
>
> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> ---
> net/wireless/nl80211.c | 80 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 61 insertions(+), 19 deletions(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index ff760ba83449..26bab9560c0f 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -13587,6 +13587,7 @@ static int nl80211_probe_mesh_link(struct sk_buff *skb, struct genl_info *info)
> #define NL80211_FLAG_NEED_WDEV_UP (NL80211_FLAG_NEED_WDEV |\
> NL80211_FLAG_CHECK_NETDEV_UP)
> #define NL80211_FLAG_CLEAR_SKB 0x20
> +#define NL80211_FLAG_OWNER_ONLY 0x40
>
> static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
> struct genl_info *info)
> @@ -13595,6 +13596,7 @@ static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
> struct wireless_dev *wdev;
> struct net_device *dev;
> bool rtnl = ops->internal_flags & NL80211_FLAG_NEED_RTNL;
> + int ret;
>
> if (rtnl)
> rtnl_lock();
> @@ -13602,10 +13604,10 @@ static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
> if (ops->internal_flags & NL80211_FLAG_NEED_WIPHY) {
> rdev = cfg80211_get_dev_from_info(genl_info_net(info), info);
> if (IS_ERR(rdev)) {
> - if (rtnl)
> - rtnl_unlock();
> - return PTR_ERR(rdev);
> + ret = PTR_ERR(rdev);
> + goto done;
> }
> +
> info->user_ptr[0] = rdev;
> } else if (ops->internal_flags & NL80211_FLAG_NEED_NETDEV ||
> ops->internal_flags & NL80211_FLAG_NEED_WDEV) {
> @@ -13614,32 +13616,33 @@ static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
> wdev = __cfg80211_wdev_from_attrs(genl_info_net(info),
> info->attrs);
> if (IS_ERR(wdev)) {
> - if (rtnl)
> - rtnl_unlock();
> - return PTR_ERR(wdev);
> + ret = PTR_ERR(wdev);
> + goto done;
> }
>
> dev = wdev->netdev;
> rdev = wiphy_to_rdev(wdev->wiphy);
>
> + ret = -EINVAL;
> if (ops->internal_flags & NL80211_FLAG_NEED_NETDEV) {
> - if (!dev) {
> - if (rtnl)
> - rtnl_unlock();
> - return -EINVAL;
> - }
> + if (!dev)
> + goto done;
>
> info->user_ptr[1] = dev;
> } else {
> info->user_ptr[1] = wdev;
> }
>
> + ret = -ENETDOWN;
> if (ops->internal_flags & NL80211_FLAG_CHECK_NETDEV_UP &&
> - !wdev_running(wdev)) {
> - if (rtnl)
> - rtnl_unlock();
> - return -ENETDOWN;
> - }
> + !wdev_running(wdev))
> + goto done;
> +
> + ret = -EPERM;
> + if (ops->internal_flags & NL80211_FLAG_OWNER_ONLY &&
> + wdev->owner_nlportid &&
> + wdev->owner_nlportid != info->snd_portid)
> + goto done;
>
> if (dev)
> dev_hold(dev);
> @@ -13647,7 +13650,13 @@ static int nl80211_pre_doit(const struct genl_ops *ops, struct sk_buff *skb,
> info->user_ptr[0] = rdev;
> }
>
> - return 0;
> + ret = 0;
> +
> +done:
> + if (rtnl && !ret)
> + rtnl_unlock();
> +
> + return ret;
> }
>
> static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
> @@ -13712,7 +13721,8 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_set_interface,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV |
> - NL80211_FLAG_NEED_RTNL,
> + NL80211_FLAG_NEED_RTNL |
> + NL80211_FLAG_OWNER_ONLY,
> },
> {
> .cmd = NL80211_CMD_NEW_INTERFACE,
> @@ -13728,7 +13738,8 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_del_interface,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV |
> - NL80211_FLAG_NEED_RTNL,
> + NL80211_FLAG_NEED_RTNL |
> + NL80211_FLAG_OWNER_ONLY,
> },
> {
> .cmd = NL80211_CMD_GET_KEY,
> @@ -13745,6 +13756,7 @@ static const struct genl_ops nl80211_ops[] = {
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> NL80211_FLAG_NEED_RTNL |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_CLEAR_SKB,
> },
> {
> @@ -13754,6 +13766,7 @@ static const struct genl_ops nl80211_ops[] = {
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> NL80211_FLAG_NEED_RTNL |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_CLEAR_SKB,
> },
> {
> @@ -13762,6 +13775,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_del_key,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13778,6 +13792,7 @@ static const struct genl_ops nl80211_ops[] = {
> .flags = GENL_UNS_ADMIN_PERM,
> .doit = nl80211_start_ap,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13786,6 +13801,7 @@ static const struct genl_ops nl80211_ops[] = {
> .flags = GENL_UNS_ADMIN_PERM,
> .doit = nl80211_stop_ap,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13802,6 +13818,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_set_station,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13810,6 +13827,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_new_station,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13818,6 +13836,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_del_station,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13921,6 +13940,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_trigger_scan,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13929,6 +13949,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_abort_scan,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13942,6 +13963,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_start_sched_scan,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13950,6 +13972,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_stop_sched_scan,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13958,6 +13981,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_authenticate,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL |
> NL80211_FLAG_CLEAR_SKB,
> },
> @@ -13967,6 +13991,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_associate,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL |
> NL80211_FLAG_CLEAR_SKB,
> },
> @@ -13976,6 +14001,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_deauthenticate,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13984,6 +14010,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_disassociate,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -13992,6 +14019,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_join_ibss,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14000,6 +14028,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_leave_ibss,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> #ifdef CONFIG_NL80211_TESTMODE
> @@ -14019,6 +14048,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_connect,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL |
> NL80211_FLAG_CLEAR_SKB,
> },
> @@ -14028,6 +14058,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_update_connect_params,
> .flags = GENL_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL |
> NL80211_FLAG_CLEAR_SKB,
> },
> @@ -14037,6 +14068,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_disconnect,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14083,6 +14115,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_remain_on_channel,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14091,6 +14124,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_cancel_remain_on_channel,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14115,6 +14149,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_tx_mgmt,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14123,6 +14158,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_tx_mgmt_cancel_wait,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14147,6 +14183,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_set_cqm,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14221,6 +14258,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_set_rekey_data,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL |
> NL80211_FLAG_CLEAR_SKB,
> },
> @@ -14278,6 +14316,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_start_p2p_device,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14286,6 +14325,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_stop_p2p_device,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14371,6 +14411,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_crit_protocol_start,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
> @@ -14379,6 +14420,7 @@ static const struct genl_ops nl80211_ops[] = {
> .doit = nl80211_crit_protocol_stop,
> .flags = GENL_UNS_ADMIN_PERM,
> .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
> + NL80211_FLAG_OWNER_ONLY |
> NL80211_FLAG_NEED_RTNL,
> },
> {
>
next prev parent reply other threads:[~2019-06-21 8:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-20 22:07 [PATCH v2 1/3] nl80211: Update uapi for CMD_FRAME_WAIT_CANCEL Denis Kenzior
2019-06-20 22:07 ` [PATCH v2 2/3] nl80211: Limit certain commands to interface owner Denis Kenzior
2019-06-21 8:09 ` Arend Van Spriel [this message]
2019-06-21 13:27 ` Marcel Holtmann
2019-06-21 17:14 ` Denis Kenzior
2019-06-21 21:16 ` Arend Van Spriel
2019-06-21 22:33 ` Denis Kenzior
2019-06-22 13:44 ` Marcel Holtmann
2019-06-24 8:39 ` Arend Van Spriel
2019-06-24 17:36 ` Denis Kenzior
2019-06-20 22:07 ` [PATCH v2 3/3] nl80211: Include wiphy address setup in NEW_WIPHY Denis Kenzior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11852f40-67e5-9122-7d82-077bdd0b014a@broadcom.com \
--to=arend.vanspriel@broadcom.com \
--cc=denkenz@gmail.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox