Having a bad day, screwed up in this v3. Sorry. Ignore again. :(
On Fri, Feb 19, 2016 at 2:55 PM, Nick Lowe <nick.lowe@xxxxxxxxxxxx> wrote:
> Add rx_bytes64 and tx_bytes64 and populate them using
> NL80211_STA_INFO_RX_BYTES64 and NL80211_STA_INFO_TX_BYTES64 where support for
> these are available. This resolves the race vulnerable 32-bit value
> wrap/overflow. Rework RADIUS accounting to use these for Acct-Input-Octets,
> Acct-Input-Gigawords, Acct-Output-Octets and Acct-Output-Gigawords, these
> values are often used for billing purposes.
>
> Signed-off-by: Nick Lowe <nick.lowe@xxxxxxxxxxxx>
> ---
> src/ap/accounting.c | 75 ++++++++++++++++++++++++++------------------
> src/ap/ctrl_iface_ap.c | 7 +++--
> src/ap/sta_info.h | 10 +++---
> src/drivers/driver.h | 1 +
> src/drivers/driver_hostap.c | 14 +++++----
> src/drivers/driver_nl80211.c | 6 ++++
> 6 files changed, 70 insertions(+), 43 deletions(-)
>
> diff --git a/src/ap/accounting.c b/src/ap/accounting.c
> index f3ce121..ae6d26b 100644
> --- a/src/ap/accounting.c
> +++ b/src/ap/accounting.c
> @@ -167,19 +167,30 @@ static int accounting_sta_update_stats(struct
> hostapd_data *hapd,
> if (hostapd_drv_read_sta_data(hapd, data, sta->addr))
> return -1;
>
> - if (sta->last_rx_bytes > data->rx_bytes)
> - sta->acct_input_gigawords++;
> - if (sta->last_tx_bytes > data->tx_bytes)
> - sta->acct_output_gigawords++;
> - sta->last_rx_bytes = data->rx_bytes;
> - sta->last_tx_bytes = data->tx_bytes;
> + if (sta->last_rx_bytes_lo > data->rx_bytes)
> + sta->last_rx_bytes_hi++;
> + sta->last_rx_bytes_lo = data->rx_bytes;
> +
> + if (sta->last_tx_bytes_lo > data->tx_bytes)
> + sta->last_tx_bytes_hi++;
> + sta->last_tx_bytes_lo = data->tx_bytes;
> +
> + sta->last_rx_bytes64 = data->rx_bytes64;
> + sta->last_tx_bytes64 = data->tx_bytes64;
>
> hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_RADIUS,
> HOSTAPD_LEVEL_DEBUG, "updated TX/RX stats: "
> - "Acct-Input-Octets=%lu Acct-Input-Gigawords=%u "
> - "Acct-Output-Octets=%lu Acct-Output-Gigawords=%u",
> - sta->last_rx_bytes, sta->acct_input_gigawords,
> - sta->last_tx_bytes, sta->acct_output_gigawords);
> + "last_rx_bytes_lo=%lu last_rx_bytes_hi=%lu "
> + "last_tx_bytes_lo=%lu last_tx_bytes_hi=%lu "
> + "last_rx_bytes64=%llu "
> + "last_tx_bytes64=%llu",
> + (unsigned long)sta->last_rx_bytes_lo,
> + (unsigned long)(sta->last_rx_bytes_hi >> 32),
> + (unsigned long)sta->last_tx_bytes_lo,
> + (unsigned long)(sta->last_tx_bytes_hi >> 32),
> + (unsigned long long)sta->last_rx_bytes64,
> + (unsigned long long)sta->last_tx_bytes64
> + );
>
> return 0;
> }
> @@ -224,8 +235,12 @@ void accounting_sta_start(struct hostapd_data
> *hapd, struct sta_info *sta)
> (long unsigned int) sta->acct_session_id);
>
> os_get_reltime(&sta->acct_session_start);
> - sta->last_rx_bytes = sta->last_tx_bytes = 0;
> - sta->acct_input_gigawords = sta->acct_output_gigawords = 0;
> + sta->last_rx_bytes_hi = 0;
> + sta->last_rx_bytes_lo = 0;
> + sta->last_tx_bytes_hi = 0;
> + sta->last_tx_bytes_lo = 0;
> + sta->last_rx_bytes64 = 0;
> + sta->last_tx_bytes64 = 0;
> hostapd_drv_sta_clear_stats(hapd, sta->addr);
>
> if (!hapd->conf->radius->acct_server)
> @@ -254,7 +269,7 @@ static void accounting_sta_report(struct hostapd_data *hapd,
> int cause = sta->acct_terminate_cause;
> struct hostap_sta_driver_data data;
> struct os_reltime now_r, diff;
> - u32 gigawords;
> + unsigned long long bytes;
>
> if (!hapd->conf->radius->acct_server)
> return;
> @@ -288,37 +303,35 @@ static void accounting_sta_report(struct
> hostapd_data *hapd,
> wpa_printf(MSG_INFO, "Could not add Acct-Output-Packets");
> goto fail;
> }
> + if (data.rx_bytes64 > 0)
> + bytes = data.rx_bytes64;
> + else
> + bytes = ((unsigned long long)sta->last_rx_bytes_hi << 32)
> | data.rx_bytes;
> if (!radius_msg_add_attr_int32(msg,
> RADIUS_ATTR_ACCT_INPUT_OCTETS,
> - data.rx_bytes)) {
> + (u32)bytes)) {
> wpa_printf(MSG_INFO, "Could not add Acct-Input-Octets");
> goto fail;
> }
> - gigawords = sta->acct_input_gigawords;
> -#if __WORDSIZE == 64
> - gigawords += data.rx_bytes >> 32;
> -#endif
> - if (gigawords &&
> - !radius_msg_add_attr_int32(
> - msg, RADIUS_ATTR_ACCT_INPUT_GIGAWORDS,
> - gigawords)) {
> + if (!radius_msg_add_attr_int32(msg,
> + RADIUS_ATTR_ACCT_INPUT_GIGAWORDS,
> + (u32)(bytes >> 32))) {
> wpa_printf(MSG_INFO, "Could not add Acct-Input-Gigawords");
> goto fail;
> }
> + if (data.tx_bytes64 > 0)
> + bytes = data.tx_bytes64;
> + else
> + bytes = ((unsigned long long)sta->last_tx_bytes_hi << 32)
> | data.tx_bytes;
> if (!radius_msg_add_attr_int32(msg,
> RADIUS_ATTR_ACCT_OUTPUT_OCTETS,
> - data.tx_bytes)) {
> + (u32)bytes)) {
> wpa_printf(MSG_INFO, "Could not add Acct-Output-Octets");
> goto fail;
> }
> - gigawords = sta->acct_output_gigawords;
> -#if __WORDSIZE == 64
> - gigawords += data.tx_bytes >> 32;
> -#endif
> - if (gigawords &&
> - !radius_msg_add_attr_int32(
> - msg, RADIUS_ATTR_ACCT_OUTPUT_GIGAWORDS,
> - gigawords)) {
> + if (!radius_msg_add_attr_int32(msg,
> + RADIUS_ATTR_ACCT_OUTPUT_GIGAWORDS,
> + (u32)(bytes >> 32))) {
> wpa_printf(MSG_INFO, "Could not add Acct-Output-Gigawords");
> goto fail;
> }
> diff --git a/src/ap/ctrl_iface_ap.c b/src/ap/ctrl_iface_ap.c
> index c98978f..bce48a7 100644
> --- a/src/ap/ctrl_iface_ap.c
> +++ b/src/ap/ctrl_iface_ap.c
> @@ -35,9 +35,12 @@ static int hostapd_get_sta_tx_rx(struct hostapd_data *hapd,
> return 0;
>
> ret = os_snprintf(buf, buflen, "rx_packets=%lu\ntx_packets=%lu\n"
> - "rx_bytes=%lu\ntx_bytes=%lu\n",
> + "rx_bytes=%lu\ntx_bytes=%lu\n"
> + "rx_bytes64=%llu\ntx_bytes64=%llu\n",
> data.rx_packets, data.tx_packets,
> - data.rx_bytes, data.tx_bytes);
> + data.rx_bytes, data.tx_bytes,
> + data.rx_bytes64, data.tx_bytes64
> + );
> if (os_snprintf_error(buflen, ret))
> return 0;
> return ret;
> diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
> index 359f480..d818afb 100644
> --- a/src/ap/sta_info.h
> +++ b/src/ap/sta_info.h
> @@ -107,10 +107,12 @@ struct sta_info {
> int acct_terminate_cause; /* Acct-Terminate-Cause */
> int acct_interim_interval; /* Acct-Interim-Interval */
>
> - unsigned long last_rx_bytes;
> - unsigned long last_tx_bytes;
> - u32 acct_input_gigawords; /* Acct-Input-Gigawords */
> - u32 acct_output_gigawords; /* Acct-Output-Gigawords */
> + unsigned long last_rx_bytes_hi;
> + unsigned long last_rx_bytes_lo;
> + unsigned long last_tx_bytes_hi;
> + unsigned long last_tx_bytes_lo;
> + unsigned long long last_rx_bytes64;
> + unsigned long long last_tx_bytes64;
>
> u8 *challenge; /* IEEE 802.11 Shared Key Authentication Challenge */
>
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index a4f6704..c2a81fe 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -1377,6 +1377,7 @@ struct hostap_sta_driver_data {
> unsigned long tx_retry_count;
> int last_rssi;
> int last_ack_rssi;
> + unsigned long long rx_bytes64, tx_bytes64;
> };
>
> struct hostapd_sta_add_params {
> diff --git a/src/drivers/driver_hostap.c b/src/drivers/driver_hostap.c
> index 517a3bb..e1b7dc9 100644
> --- a/src/drivers/driver_hostap.c
> +++ b/src/drivers/driver_hostap.c
> @@ -579,7 +579,6 @@ static int hostap_read_sta_data(void *priv,
> struct hostap_driver_data *drv = priv;
> char buf[1024], line[128], *pos;
> FILE *f;
> - unsigned long val;
>
> memset(data, 0, sizeof(*data));
> snprintf(buf, sizeof(buf), "/proc/net/hostap/%s/" MACSTR,
> @@ -597,15 +596,18 @@ static int hostap_read_sta_data(void *priv,
> if (!pos)
> continue;
> *pos++ = '\0';
> - val = strtoul(pos, NULL, 10);
> if (strcmp(line, "rx_packets") == 0)
> - data->rx_packets = val;
> + data->rx_packets = strtoul(pos, NULL, 10);
> else if (strcmp(line, "tx_packets") == 0)
> - data->tx_packets = val;
> + data->tx_packets = strtoul(pos, NULL, 10);
> else if (strcmp(line, "rx_bytes") == 0)
> - data->rx_bytes = val;
> + data->rx_bytes = strtoul(pos, NULL, 10);
> else if (strcmp(line, "tx_bytes") == 0)
> - data->tx_bytes = val;
> + data->tx_bytes = strtoul(pos, NULL, 10);
> + else if (strcmp(line, "rx_bytes64") == 0)
> + data->rx_bytes64 = strtoull(pos, NULL, 20);
> + else if (strcmp(line, "tx_bytes64") == 0)
> + data->tx_bytes64 = strtoull(pos, NULL, 20);
> }
>
> fclose(f);
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 0f312a0..9916bc6 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -5327,6 +5327,8 @@ static int get_sta_handler(struct nl_msg *msg, void *arg)
> [NL80211_STA_INFO_RX_PACKETS] = { .type = NLA_U32 },
> [NL80211_STA_INFO_TX_PACKETS] = { .type = NLA_U32 },
> [NL80211_STA_INFO_TX_FAILED] = { .type = NLA_U32 },
> + [NL80211_STA_INFO_RX_BYTES64] = { .type = NLA_U64 },
> + [NL80211_STA_INFO_TX_BYTES64] = { .type = NLA_U64 },
> };
>
> nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
> @@ -5365,6 +5367,10 @@ static int get_sta_handler(struct nl_msg *msg, void *arg)
> if (stats[NL80211_STA_INFO_TX_FAILED])
> data->tx_retry_failed =
> nla_get_u32(stats[NL80211_STA_INFO_TX_FAILED]);
> + if (stats[NL80211_STA_INFO_RX_BYTES64])
> + data->rx_bytes64 = nla_get_u64(stats[NL80211_STA_INFO_RX_BYTES64]);
> + if (stats[NL80211_STA_INFO_TX_BYTES64])
> + data->tx_bytes64 = nla_get_u64(stats[NL80211_STA_INFO_TX_BYTES64]);
>
> return NL_SKIP;
> }
> --
> 2.7.1
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
