Make rx_bytes and tx_bytes 64-bit rather than 32-bit. Rework RADIUS
accounting to use these for Acct-Input-Octets, Acct-Input-Gigawords,
Acct-Output-Octets and Acct-Output-Gigawords. Remove the race
vulnerable 32-bit value wrap/overflow detection code.
Signed-off-by: Nick Lowe <nick.lowe@xxxxxxxxxxxx>
---
src/ap/accounting.c | 42 ++++++++++++++----------------------------
src/ap/ctrl_iface_ap.c | 2 +-
src/ap/sta_info.h | 6 ++----
src/drivers/driver.h | 3 ++-
src/drivers/driver_hostap.c | 10 ++++------
src/drivers/driver_nl80211.c | 12 ++++++------
6 files changed, 29 insertions(+), 46 deletions(-)
diff --git a/src/ap/accounting.c b/src/ap/accounting.c
index f3ce121..2702356 100644
--- a/src/ap/accounting.c
+++ b/src/ap/accounting.c
@@ -167,19 +167,17 @@ 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;
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);
+ "Acct-Input-Octets=%lu Acct-Input-Gigawords=%lu "
+ "Acct-Output-Octets=%lu Acct-Output-Gigawords=%lu",
+ (unsigned long)sta->last_rx_bytes,
+ (unsigned long)(sta->last_rx_bytes >> 32),
+ (unsigned long)sta->last_tx_bytes,
+ (unsigned long)(sta->last_tx_bytes >> 32));
return 0;
}
@@ -225,7 +223,6 @@ void accounting_sta_start(struct hostapd_data
*hapd, struct sta_info *sta)
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;
hostapd_drv_sta_clear_stats(hapd, sta->addr);
if (!hapd->conf->radius->acct_server)
@@ -254,7 +251,6 @@ 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;
if (!hapd->conf->radius->acct_server)
return;
@@ -290,35 +286,25 @@ static void accounting_sta_report(struct
hostapd_data *hapd,
}
if (!radius_msg_add_attr_int32(msg,
RADIUS_ATTR_ACCT_INPUT_OCTETS,
- data.rx_bytes)) {
+ (u32)data.rx_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)(data.rx_bytes >> 32))) {
wpa_printf(MSG_INFO, "Could not add Acct-Input-Gigawords");
goto fail;
}
if (!radius_msg_add_attr_int32(msg,
RADIUS_ATTR_ACCT_OUTPUT_OCTETS,
- data.tx_bytes)) {
+ (u32)data.tx_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)(data.tx_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..4f93ac2 100644
--- a/src/ap/ctrl_iface_ap.c
+++ b/src/ap/ctrl_iface_ap.c
@@ -35,7 +35,7 @@ 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=%llu\ntx_bytes=%llu\n",
data.rx_packets, data.tx_packets,
data.rx_bytes, data.tx_bytes);
if (os_snprintf_error(buflen, ret))
diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
index 359f480..114ba65 100644
--- a/src/ap/sta_info.h
+++ b/src/ap/sta_info.h
@@ -107,10 +107,8 @@ 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 long last_rx_bytes;
+ unsigned long long last_tx_bytes;
u8 *challenge; /* IEEE 802.11 Shared Key Authentication Challenge */
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index aad0e04..8f55478 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1356,7 +1356,8 @@ struct wpa_driver_capa {
struct hostapd_data;
struct hostap_sta_driver_data {
- unsigned long rx_packets, tx_packets, rx_bytes, tx_bytes;
+ unsigned long rx_packets, tx_packets;
+ unsigned long long rx_bytes, tx_bytes;
unsigned long current_tx_rate;
unsigned long inactive_msec;
unsigned long flags;
diff --git a/src/drivers/driver_hostap.c b/src/drivers/driver_hostap.c
index 517a3bb..6e43cef 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,14 @@ 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 = strtoull(pos, NULL, 20);
else if (strcmp(line, "tx_bytes") == 0)
- data->tx_bytes = val;
+ data->tx_bytes = strtoull(pos, NULL, 20);
}
fclose(f);
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 4cb92d0..7c861b6 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -5310,8 +5310,8 @@ static int get_sta_handler(struct nl_msg *msg, void *arg)
struct nlattr *stats[NL80211_STA_INFO_MAX + 1];
static struct nla_policy stats_policy[NL80211_STA_INFO_MAX + 1] = {
[NL80211_STA_INFO_INACTIVE_TIME] = { .type = NLA_U32 },
- [NL80211_STA_INFO_RX_BYTES] = { .type = NLA_U32 },
- [NL80211_STA_INFO_TX_BYTES] = { .type = NLA_U32 },
+ [NL80211_STA_INFO_RX_BYTES64] = { .type = NLA_U64 },
+ [NL80211_STA_INFO_TX_BYTES64] = { .type = NLA_U64 },
[NL80211_STA_INFO_RX_PACKETS] = { .type = NLA_U32 },
[NL80211_STA_INFO_TX_PACKETS] = { .type = NLA_U32 },
[NL80211_STA_INFO_TX_FAILED] = { .type = NLA_U32 },
@@ -5340,10 +5340,10 @@ static int get_sta_handler(struct nl_msg *msg,
void *arg)
if (stats[NL80211_STA_INFO_INACTIVE_TIME])
data->inactive_msec =
nla_get_u32(stats[NL80211_STA_INFO_INACTIVE_TIME]);
- if (stats[NL80211_STA_INFO_RX_BYTES])
- data->rx_bytes = nla_get_u32(stats[NL80211_STA_INFO_RX_BYTES]);
- if (stats[NL80211_STA_INFO_TX_BYTES])
- data->tx_bytes = nla_get_u32(stats[NL80211_STA_INFO_TX_BYTES]);
+ if (stats[NL80211_STA_INFO_RX_BYTES64])
+ data->rx_bytes = nla_get_u64(stats[NL80211_STA_INFO_RX_BYTES64]);
+ if (stats[NL80211_STA_INFO_TX_BYTES64])
+ data->tx_bytes = nla_get_u64(stats[NL80211_STA_INFO_TX_BYTES64]);
if (stats[NL80211_STA_INFO_RX_PACKETS])
data->rx_packets =
nla_get_u32(stats[NL80211_STA_INFO_RX_PACKETS]);
--
2.5.0
From 24aa94e510fea5fcafdbe91f45f7a21c46aeebd4 Mon Sep 17 00:00:00 2001
From: Nick Lowe <nick.lowe@xxxxxxxxxxxx>
Date: Sat, 13 Feb 2016 14:55:56 +0000
Subject: [PATCH] Make rx_bytes and tx_bytes 64-bit rather than 32-bit. Rework
RADIUS accounting to use these for Acct-Input-Octets, Acct-Input-Gigawords,
Acct-Output-Octets and Acct-Output-Gigawords. Remove the race vulnerable
32-bit value wrap/overflow detection code.
Signed-off-by: Nick Lowe <nick.lowe@xxxxxxxxxxxx>
---
src/ap/accounting.c | 42 ++++++++++++++----------------------------
src/ap/ctrl_iface_ap.c | 2 +-
src/ap/sta_info.h | 6 ++----
src/drivers/driver.h | 3 ++-
src/drivers/driver_hostap.c | 10 ++++------
src/drivers/driver_nl80211.c | 12 ++++++------
6 files changed, 29 insertions(+), 46 deletions(-)
diff --git a/src/ap/accounting.c b/src/ap/accounting.c
index f3ce121..2702356 100644
--- a/src/ap/accounting.c
+++ b/src/ap/accounting.c
@@ -167,19 +167,17 @@ 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;
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);
+ "Acct-Input-Octets=%lu Acct-Input-Gigawords=%lu "
+ "Acct-Output-Octets=%lu Acct-Output-Gigawords=%lu",
+ (unsigned long)sta->last_rx_bytes,
+ (unsigned long)(sta->last_rx_bytes >> 32),
+ (unsigned long)sta->last_tx_bytes,
+ (unsigned long)(sta->last_tx_bytes >> 32));
return 0;
}
@@ -225,7 +223,6 @@ void accounting_sta_start(struct hostapd_data *hapd, struct sta_info *sta)
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;
hostapd_drv_sta_clear_stats(hapd, sta->addr);
if (!hapd->conf->radius->acct_server)
@@ -254,7 +251,6 @@ 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;
if (!hapd->conf->radius->acct_server)
return;
@@ -290,35 +286,25 @@ static void accounting_sta_report(struct hostapd_data *hapd,
}
if (!radius_msg_add_attr_int32(msg,
RADIUS_ATTR_ACCT_INPUT_OCTETS,
- data.rx_bytes)) {
+ (u32)data.rx_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)(data.rx_bytes >> 32))) {
wpa_printf(MSG_INFO, "Could not add Acct-Input-Gigawords");
goto fail;
}
if (!radius_msg_add_attr_int32(msg,
RADIUS_ATTR_ACCT_OUTPUT_OCTETS,
- data.tx_bytes)) {
+ (u32)data.tx_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)(data.tx_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..4f93ac2 100644
--- a/src/ap/ctrl_iface_ap.c
+++ b/src/ap/ctrl_iface_ap.c
@@ -35,7 +35,7 @@ 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=%llu\ntx_bytes=%llu\n",
data.rx_packets, data.tx_packets,
data.rx_bytes, data.tx_bytes);
if (os_snprintf_error(buflen, ret))
diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
index 359f480..114ba65 100644
--- a/src/ap/sta_info.h
+++ b/src/ap/sta_info.h
@@ -107,10 +107,8 @@ 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 long last_rx_bytes;
+ unsigned long long last_tx_bytes;
u8 *challenge; /* IEEE 802.11 Shared Key Authentication Challenge */
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index aad0e04..8f55478 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1356,7 +1356,8 @@ struct wpa_driver_capa {
struct hostapd_data;
struct hostap_sta_driver_data {
- unsigned long rx_packets, tx_packets, rx_bytes, tx_bytes;
+ unsigned long rx_packets, tx_packets;
+ unsigned long long rx_bytes, tx_bytes;
unsigned long current_tx_rate;
unsigned long inactive_msec;
unsigned long flags;
diff --git a/src/drivers/driver_hostap.c b/src/drivers/driver_hostap.c
index 517a3bb..6e43cef 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,14 @@ 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 = strtoull(pos, NULL, 20);
else if (strcmp(line, "tx_bytes") == 0)
- data->tx_bytes = val;
+ data->tx_bytes = strtoull(pos, NULL, 20);
}
fclose(f);
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 4cb92d0..7c861b6 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -5310,8 +5310,8 @@ static int get_sta_handler(struct nl_msg *msg, void *arg)
struct nlattr *stats[NL80211_STA_INFO_MAX + 1];
static struct nla_policy stats_policy[NL80211_STA_INFO_MAX + 1] = {
[NL80211_STA_INFO_INACTIVE_TIME] = { .type = NLA_U32 },
- [NL80211_STA_INFO_RX_BYTES] = { .type = NLA_U32 },
- [NL80211_STA_INFO_TX_BYTES] = { .type = NLA_U32 },
+ [NL80211_STA_INFO_RX_BYTES64] = { .type = NLA_U64 },
+ [NL80211_STA_INFO_TX_BYTES64] = { .type = NLA_U64 },
[NL80211_STA_INFO_RX_PACKETS] = { .type = NLA_U32 },
[NL80211_STA_INFO_TX_PACKETS] = { .type = NLA_U32 },
[NL80211_STA_INFO_TX_FAILED] = { .type = NLA_U32 },
@@ -5340,10 +5340,10 @@ static int get_sta_handler(struct nl_msg *msg, void *arg)
if (stats[NL80211_STA_INFO_INACTIVE_TIME])
data->inactive_msec =
nla_get_u32(stats[NL80211_STA_INFO_INACTIVE_TIME]);
- if (stats[NL80211_STA_INFO_RX_BYTES])
- data->rx_bytes = nla_get_u32(stats[NL80211_STA_INFO_RX_BYTES]);
- if (stats[NL80211_STA_INFO_TX_BYTES])
- data->tx_bytes = nla_get_u32(stats[NL80211_STA_INFO_TX_BYTES]);
+ if (stats[NL80211_STA_INFO_RX_BYTES64])
+ data->rx_bytes = nla_get_u64(stats[NL80211_STA_INFO_RX_BYTES64]);
+ if (stats[NL80211_STA_INFO_TX_BYTES64])
+ data->tx_bytes = nla_get_u64(stats[NL80211_STA_INFO_TX_BYTES64]);
if (stats[NL80211_STA_INFO_RX_PACKETS])
data->rx_packets =
nla_get_u32(stats[NL80211_STA_INFO_RX_PACKETS]);
--
2.5.0
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
