From 21f33abfa72c92010b379ccf7feccee97ec60ec4 Mon Sep 17 00:00:00 2001 From: Erdnusschokolade <96622762+Erdnusschokolade@users.noreply.github.com> Date: Thu, 21 May 2026 17:39:29 +0200 Subject: [PATCH 1/2] Fix VLAN name persistence across rename and delete operations Previously, renaming a VLAN or deleting and recreating it with a different name did not update the displayed name. The vlan_names[] array is an append-only buffer where vlan_name() returns the first matching entry, so stale entries kept winning. This commit adds vlan_name_remove(), which locates an entry by VLAN ID and removes it via array compaction. The function is called in two places: - parse_vlan() in cmd_parser.c, before appending a new name entry, to remove any pre-existing entry for the same VLAN ID - vlan_delete() in rtl837x_port.c, to clean up the name when a VLAN is removed The implementation reuses the existing vlan_name() lookup, scans for the trailing space of the matched entry, then shifts remaining bytes left. Locals are declared as static __xdata to avoid the SDCC overlay segment limit on banked functions. Tested on KeepLiNK KP-9000-6XH-X: - vlan 99 AAA p1u; vlan 99 BBB -> name updated to BBB - vlan 99 d; vlan 99 CCC p1u -> name correctly CCC, not stale AAA Note: This fix addresses the runtime XMEM state. Persistence of renamed VLAN names across reboot requires the user to download and re-upload /config, as is the existing pattern for all configuration changes in this firmware. --- cmd_parser.c | 1 + rtl837x_port.c | 34 ++++++++++++++++++++++++++++++++++ rtl837x_port.h | 1 + 3 files changed, 36 insertions(+) diff --git a/cmd_parser.c b/cmd_parser.c index 4d2e427..7f4004a 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -347,6 +347,7 @@ void parse_vlan(void) uint8_t w = 2; if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { register uint8_t i = 0; + vlan_name_remove(vlan_settings.vlan); vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; vlan_names[vlan_ptr++] = hex[vlan_settings.vlan & 0xf]; diff --git a/rtl837x_port.c b/rtl837x_port.c index 7c48f90..c540ac3 100644 --- a/rtl837x_port.c +++ b/rtl837x_port.c @@ -116,11 +116,45 @@ uint16_t port_pvid_get(uint8_t port) __banked void vlan_delete(uint16_t vlan) __banked { print_string("\nvlan_delete called \n"); print_short(vlan); + vlan_name_remove(vlan); REG_WRITE(RTL837x_TBL_DATA_IN_A, 0, 0, 0, 0); REG_WRITE(RTL837X_TBL_CTRL, vlan >> 8, vlan, TBL_VLAN, TBL_WRITE | TBL_EXECUTE); } +void vlan_name_remove(uint16_t vlan) __banked +{ + static __xdata uint16_t name_pos; + static __xdata uint16_t entry_start; + static __xdata uint16_t pos; + static __xdata uint16_t entry_len; + static __xdata uint16_t move_count; + static __xdata uint16_t j; + + name_pos = vlan_name(vlan); + if (name_pos == 0xffff) + return; + + entry_start = name_pos - 3; + pos = entry_start; + + while (pos < vlan_ptr && vlan_names[pos] != ' ') + pos++; + if (pos >= vlan_ptr) + return; + pos++; + + entry_len = pos - entry_start; + move_count = vlan_ptr - pos; + + for (j = 0; j < move_count; j++) + vlan_names[entry_start + j] = vlan_names[pos + j]; + + vlan_ptr -= entry_len; + vlan_names[vlan_ptr] = 0; +} + + /* * Reads VLAN information from VLAN table * Returns data in sfr_data diff --git a/rtl837x_port.h b/rtl837x_port.h index ff036b9..a5ce278 100644 --- a/rtl837x_port.h +++ b/rtl837x_port.h @@ -50,6 +50,7 @@ void port_l2_learned(void) __banked; void port_stats_print(void) __banked; int8_t vlan_get(register uint16_t vlan) __banked; __xdata uint16_t vlan_name(register uint16_t vlan) __banked; +void vlan_name_remove(uint16_t vlan) __banked; void vlan_setup(void) __banked; void port_pvid_set(uint8_t port, __xdata uint16_t pvid) __banked; uint16_t port_pvid_get(uint8_t port) __banked; From 7d0d8525d29048a0ddc34ca1f7281b6bf1043f8e Mon Sep 17 00:00:00 2001 From: Erdnusschokolade <96622762+Erdnusschokolade@users.noreply.github.com> Date: Sat, 23 May 2026 11:19:42 +0200 Subject: [PATCH 2/2] Fix two OOB reads in parse_vlan() 1. While loop scanning VLAN name terminated only on ' ', not '\0'. When the name is the last token in cmd_buffer, the loop reads past the buffer into adjacent XRAM. 2. Entering 'vlan' without arguments causes parse_vlan() to read cmd_words_b[1] which points to undefined memory, causing atoi_short() to interpret residual bytes from previous commands as a VLAN ID. Bug found and fix proposed by logicog during review of PR #232. Co-Authored-By: logicog --- cmd_parser.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd_parser.c b/cmd_parser.c index 7f4004a..ba9ceb3 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -331,6 +331,8 @@ void parse_vlan(void) vlan_settings.vlan = 0; vlan_settings.members = 0; vlan_settings.tagged = 0; + if (cmd_words_len < 2) + goto err; if (!atoi_short(&vlan_settings.vlan, cmd_words_b[1])) { if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { vlan_delete(vlan_settings.vlan); @@ -351,7 +353,7 @@ void parse_vlan(void) vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; vlan_names[vlan_ptr++] = hex[vlan_settings.vlan & 0xf]; - while(cmd_buffer[cmd_words_b[w] + i] != ' ') { + while(cmd_buffer[cmd_words_b[w] + i] != ' ' && cmd_buffer[cmd_words_b[w] + i] != '\0') { write_char(cmd_buffer[cmd_words_b[w] + i]); vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; }