| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAELVAF+-6F0Qvj3wU-sO0+EF5Qs0VXYqD9GEeV0ZuP-dKOoFA@mail.gmail.com>
Date: Tue, 20 Nov 2018 19:20:01 -0800
From: Cody Schuffelen <schuffelen@...gle.com>
To: sergey.matyukevich.os@...ntenna.com
Cc: Johannes Berg <johannes@...solutions.net>,
Kalle Valo <kvalo@...eaurora.org>,
"David S. Miller" <davem@...emloft.net>,
linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
netdev@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH net-next v3] wireless-drivers: rtnetlink wifi simulation device
> > + informed_bss =
> > + cfg80211_inform_bss_data(wiphy, &mock_inform_bss,
> > + CFG80211_BSS_FTYPE_PRESP,
> > + fake_router_bssid,
> > + mock_inform_bss.boottime_ns,
> > + WLAN_CAPABILITY_ESS, 0, ssid.data,
> > + sizeof(ssid), GFP_KERNEL);
>
> It is possible to simplify this part switching to cfg80211_inform_bss
> function: this function wraps your scan data in into cfg80211_inform_bss
> structure internally using some reasonable defaults, e.g. channel width.
>
> Besides, signal strength for scan entries should be passed in mBm units,
> so use DBM_TO_MBM macro. For instance, with your current code 'iw' tool
> produces the following output:
> $ sudo iw dev wlan0 scan
> ...
> signal: 0.-60 dBm
> ...
Good catch, fixed.
> > +static void virt_wifi_connect_complete(struct work_struct *work)
> > +{
> > + struct virt_wifi_priv *priv =
> > + container_of(work, struct virt_wifi_priv, connect.work);
> > + u8 *requested_bss = priv->connect_requested_bss;
> > + bool has_addr = !is_zero_ether_addr(requested_bss);
> > + bool right_addr = ether_addr_equal(requested_bss, fake_router_bssid);
> > + u16 status = WLAN_STATUS_SUCCESS;
> > +
> > + rtnl_lock();
> > + if (!priv->netdev_is_up || (has_addr && !right_addr))
> > + status = WLAN_STATUS_UNSPECIFIED_FAILURE;
> > + else
> > + priv->is_connected = true;
> > +
> > + cfg80211_connect_result(priv->netdev, requested_bss, NULL, 0, NULL, 0,
> > + status, GFP_KERNEL);
> > + rtnl_unlock();
> > +}
>
> Carrier state for wireless device depends on its connection state.
> E.g., carrier is set when wireless connection succeeds and cleared
> when device disconnects. So use netif_carrier_on/netif_carrier_off
> calls in connect/disconnect handlers to set correct carrier state.
> IIUC the following locations look reasonable:
> - netif_carrier_off on init
> - netif_carrier_on in virt_wifi_connect_complete on success
> - netif_carrier_off in virt_wifi_disconnect
Thanks, added.
> > +static void virt_wifi_disconnect_complete(struct work_struct *work)
> > +{
> > + struct virt_wifi_priv *priv =
> > + container_of(work, struct virt_wifi_priv, disconnect.work);
> > +
> > + cfg80211_disconnected(priv->netdev, priv->disconnect_reason, NULL, 0,
> > + true, GFP_KERNEL);
> > + priv->is_connected = false;
> > +}
>
> Why do you need delayed disconnect processing ? IIUC it can be dropped
> and cfg80211_disconnected call can be moved to virt_wifi_disconnect.
Done.
> > +
> > +static int virt_wifi_get_station(struct wiphy *wiphy,
> > + struct net_device *dev,
> > + const u8 *mac,
> > + struct station_info *sinfo)
> > +{
> > + wiphy_debug(wiphy, "get_station\n");
> > +
> > + if (!ether_addr_equal(mac, fake_router_bssid))
> > + return -ENOENT;
> > +
> > + sinfo->filled = BIT(NL80211_STA_INFO_TX_PACKETS) |
> > + BIT(NL80211_STA_INFO_TX_FAILED) | BIT(NL80211_STA_INFO_SIGNAL) |
> > + BIT(NL80211_STA_INFO_TX_BITRATE);
>
> Recently some of NL80211_STA_INFO_ attribute types has been modified to
> use BIT_ULL macro. Could you please check commit 22d0d2fafca93ba1d92a
> for details and modify your coded if needed.
Thanks for the the reference, updated to use BIT_ULL with the station commands.
> > + sinfo->tx_packets = 1;
>
> Only one packet, really ? Not sure if you plan to use the output of 'iw'
> or any other tool. If yes, then it probably makes sense to use stats
> from the original network link. Otherwise, your 'iw' output is
> going to look like this:
>
> $ iw dev wlan0 station dump
> ...
> tx packets: 1
> ...
>
> > + sinfo->tx_failed = 0;
>
> ...
Added bookkeeping to the net_device packet forwarded to track how many
packets were sent, and how many failed being sent due to no
connection.
> > +static int virt_wifi_dump_station(struct wiphy *wiphy,
> > + struct net_device *dev,
> > + int idx,
> > + u8 *mac,
> > + struct station_info *sinfo)
> > +{
> > + wiphy_debug(wiphy, "dump_station\n");
> > +
> > + if (idx != 0)
> > + return -ENOENT;
> > +
> > + ether_addr_copy(mac, fake_router_bssid);
> > + return virt_wifi_get_station(wiphy, dev, fake_router_bssid, sinfo);
> > +}
>
> Callback dump_station should return AP data only when STA is connected.
> Currently your driver returns fake AP data even when it is not
> connected.
Thanks, fixed.
> > +static const struct cfg80211_ops virt_wifi_cfg80211_ops = {
> > + .scan = virt_wifi_scan,
> > +
> > + .connect = virt_wifi_connect,
> > + .disconnect = virt_wifi_disconnect,
> > +
> > + .get_station = virt_wifi_get_station,
> > + .dump_station = virt_wifi_dump_station,
> > +};
>
> Hey, this minimal cfg80211 implementation works fine with wpa_supplicant
> and open AP config. By the way, if you plan to add more features, then
> I would suggest to consider the following cfg80211 callbacks:
> - change_station, get_channel
> to provide more info in connected state, e.g. compare the output
> of the following commands between your virtual interface and
> actual wireless interface:
> $ iw dev wlan0 link
> $ iw dev wlan0 info
>
> - stubs for add_key, del_key to enable encrypted AP simulation
Thanks for testing it out!
I've uploaded the new version here: https://lkml.org/lkml/2018/11/21/251
Powered by blists - more mailing lists