Skip to content

RDKB-65569 : [XB10][RDKB] WiFi management service crashes with finge… - #1309

Open
SakeVictorDaniel wants to merge 3 commits into
rdkcentral:developfrom
SakeVictorDaniel:RDKB-65569_Potential_Fix
Open

RDKB-65569 : [XB10][RDKB] WiFi management service crashes with finge…#1309
SakeVictorDaniel wants to merge 3 commits into
rdkcentral:developfrom
SakeVictorDaniel:RDKB-65569_Potential_Fix

Conversation

@SakeVictorDaniel

@SakeVictorDaniel SakeVictorDaniel commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

RDKB-65569 : WiFi management service crashes with fingerprint 58928292 on XB10 and XER10 gateways, which can cause brief WiFi interruptions for customers each time it restart

Reason for Change:

--> wifi_getApAssociatedDeviceDiagnosticResult3() was failing continuously as the driver returns early since /tmp/sta_assoc_count is greater than 8096 bytes

--> wifi_getApAssociatedDeviceDiagnosticResult3() is invoked every 5000ms.

--> When wifi_getApAssociatedDeviceDiagnosticResult3() fails, clean up logic never hits and stale entries in sta_map are never cleaned. This causes harvester_get_associated_device_info() to overflow its fixed 66,500 bytes harvester_buf when serializing all entries to JSON when populating the harvester_buf on harvester reporting interval.

Fix:

When wifi_getApAssociatedDeviceDiagnosticResult3() fails, we now still clean sta_map the same way as normal flow so stale entries do not keep growing:

  1. Old disconnected clients are removed only after they stay inactive longer than rapid_reconnect_threshold.
  2. Assoc-request-only entries (never fully connected) are removed after timeout.
  3. Disconnect events are sent only for true stale disconnected clients, not for assoc-only cleanup.
  4. Assoc timestamp aging now compares epoch time with epoch time (removed epoch vs CLOCK_MONOTONIC comparision).
  5. Harvester JSON building is made safe by:
    --> Checking remaining buffer space before each client entry.
    --> Checking snprintf return for error and truncation.
    --> Stop safely on truncation to avoid pointer jump and unsigned underflow.
    --> Keep full per-client buffer budget, reserving only exact bytes needed for JSON closing trailer.

Unit Testing:

  1. Connected 42 clients to 5GHz private SSID

  2. Confirmed that all the connected clients are listed under wl -i wl1.1 assoclist

  3. Made the /tmp/sta_assoc_count greater than 8096 to simulate the stats fetch failure (as observed in the issue devices in field)

  4. Checked /rdklogs/logs/messages.txt and observed

"2026 Jul 28 07:52:57 Docsis-Gateway kernel: CFG80211-ERROR) wl_cfgvendor_get_assoc_num : Failed to read whole file /tmp/sta_assoc_count

2026 Jul 28 07:52:57 Docsis-Gateway kernel: CFG80211-ERROR) wl_cfgvendor_get_station_reply : Failed to get assoc num, error: -1

reply handler return early due to /tmp/sta_assoc_count being more than 8098 bytes and as a result in /rdklogs/logs/wifiMon.txt observe "Failed to get AP Associated Devices statistics for vap index 1" every 5 seconds

  1. Disconnected 15 connected clients

  2. Out of 15 disconnected clients, reconnected 5 clients within rapid disconnect threshold (180s)

  3. Checked in /rdklogs/logs/wifiMon.txt.0 that exactly after 180s the 10 disconnected clients (that were not reconnected within 180s) are removed from the sta_map by checking the logs

  4. Waited for the next harvester report and saw that there is no disconnected entry in the report. But the reconnected client entries are present as expected.

  5. Left the device in this state for more than 17 hours, harvester report does not contain any stale entries.

Attaching relevant logs:

65569_Disconnected_Clients_&_Reconnected_Clients.txt

65569_RG_Console_logs.txt

harvester_buf_contents.txt

Priority: P1
Risk: Medium

Signed-off-by: Sake Victor Daniel VictorDaniel_Sake@comcast.com

Copilot AI review requested due to automatic review settings July 30, 2026 06:06
@SakeVictorDaniel
SakeVictorDaniel requested a review from a team as a code owner July 30, 2026 06:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents WiFi management service crashes caused by unbounded sta_map growth and subsequent overflow when serializing associated-client diagnostics into a fixed-size JSON buffer.

Changes:

  • Adds stale sta_map cleanup even when wifi_getApAssociatedDeviceDiagnosticResult3() fails, avoiding unbounded map growth.
  • Adds a bounds check in harvester_get_associated_device_info() to stop serializing client entries before exceeding the fixed diagnostics buffer.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
source/stats/wifi_stats_assoc_client.c Adds cleanup on HAL failure to remove stale station-map entries rather than returning early with no pruning.
source/stats/wifi_monitor.c Adds bounds checking during JSON serialization of associated-client diagnostics to prevent buffer overflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/stats/wifi_monitor.c Outdated
Comment on lines +1046 to +1054
/* Bounds check: stop serializing if we are approaching buffer limit.
* Reserve 64 bytes for the closing JSON brackets and null terminator.
*/
if (pos >= (buf_size - 64)) {
wifi_util_error_print(WIFI_MON,
"%s %d Buffer overflow prevented for vap %d, pos=%u buf_size=%u sta_count=%u\n",
__func__, __LINE__, vap_index, pos, buf_size, sta_count);
break;
}
Comment thread source/stats/wifi_stats_assoc_client.c
@SakeVictorDaniel
SakeVictorDaniel force-pushed the RDKB-65569_Potential_Fix branch from b3288da to 7c99cf3 Compare August 4, 2026 11:09
Copilot AI review requested due to automatic review settings August 4, 2026 11:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

source/stats/wifi_stats_assoc_client.c:286

  • assoc_frame_data.frame_timestamp is set with time() (epoch seconds) in wifi_monitor.c, but this cleanup compares it against tv_now_err.tv_sec from CLOCK_MONOTONIC. These clocks are not comparable, so assoc-request-only entries may never age out (or age out incorrectly after reboots). Use time()/CLOCK_REALTIME consistently for frame_timestamp comparisons.
                    if ((sta->assoc_frame_data.frame_timestamp != 0) &&
                        (tv_now_err.tv_sec - sta->assoc_frame_data.frame_timestamp > 5)) {

Comment thread source/stats/wifi_monitor.c Outdated
Comment on lines +1049 to +1057
if (pos >= (buf_size - 64)) {
wifi_util_error_print(WIFI_MON,
"%s %d Buffer overflow prevented for vap %d, pos=%u buf_size=%u sta_count=%u\n",
__func__, __LINE__, vap_index, pos, buf_size, sta_count);
break;
}
sta_count++;
pos += snprintf(&harvester_buf[vap_index][pos],
buf_size - pos, "{"
vsake057 added 2 commits August 5, 2026 07:34
…print 58928292 on XB10 and XER10 gateways, which can cause brief WiFi interruptions for customers each time it restart

Reason for Change:
When wifi_getApAssociatedDeviceDiagnosticResult3() fails,
stale entries in sta_map are never cleaned up while process_connect()
keeps adding new ones. This causes harvester_get_associated_device_info()
to overflow its fixed 66500 byte buffer when serializing all entries to JSON.

Fix:
1) Add bounds check in harvester loop to stop before overflow.
2) Perform stale entry cleanup even when HAL call fails.

Signed-off-by: Sake Victor Daniel <VictorDaniel_Sake@comcast.com>
…tries (including assoc-only)

and preserving disconnect-event behaviour

Signed-off-by: Sake Victor Daniel <VictorDaniel_Sake@comcast.com>
@SakeVictorDaniel
SakeVictorDaniel force-pushed the RDKB-65569_Potential_Fix branch from 7c99cf3 to 5bf9b14 Compare August 5, 2026 09:06
Copilot AI review requested due to automatic review settings August 5, 2026 09:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

source/stats/wifi_monitor.c:1042

  • snprintf() return value is assigned directly to unsigned pos without validating for errors or truncation. If snprintf() returns a negative value, or a value >= buf_size, later code (e.g., harvester_buf[vap_index][pos-1] and the trailer snprintf) can read/write out of bounds.
    pos = snprintf(harvester_buf[vap_index], buf_size,
                "{"
                "\"Version\":\"1.0\","
                "\"AssociatedClientsDiagnostics\":["
                "{"

Signed-off-by: Sake Victor Daniel <VictorDaniel_Sake@comcast.com>
@SakeVictorDaniel
SakeVictorDaniel force-pushed the RDKB-65569_Potential_Fix branch from 5bf9b14 to 068a32b Compare August 5, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants