nm-initrd-generator: fix IPv6 with square brackets in bond options

If any bond option contains an IPv6 address it needs to be enclosed with
[]. Otherwise the ':' separators from the IP address can be confused
with the ':' separators from the 'bond=' cmdline arguments.

However, the square brackets were ignored:
    $ nm-initrd-generator -s "bond=bond0:eth0,eth1:ns_ip6_target=[FC08::789:1:0:0:3]"
    NetworkManager-Message: 08:46:55.114: <warn>  [1745498815.1146] cmdline-reader: Ignoring invalid bond option: "ns_ip6_target" = "[FC08": '[FC08' is not a valid IPv6 address for 'ns_ip6_target' option
    NetworkManager-Message: 08:46:55.114: <warn>  [1745498815.1148] cmdline-reader: Ignoring extra: '789:1:0:0:3]'.

The opening '[' was only being considered if it was the first character
in `get_word`. Fix it and consider it if it's in the middle too.

If the brackets are used first and last, directly remove them as it is what
most callers expect. However, if it's in the middle there is no reasonable
way to remove them, so don't do it. Instead, the caller will have to consider
this possibility when processing the content.

Fixes: ecc074b2f8 ('initrd: add command line parser')
Fixes https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1755
This commit is contained in:
Íñigo Huguet
2025-04-25 14:35:02 +02:00
parent 9e585b6cdc
commit aeaf8ca23c
2 changed files with 159 additions and 23 deletions

View File

@@ -299,33 +299,44 @@ get_word(char **argument, const char separator)
{
char *word;
int nest = 0;
char *last_ch;
char *first_close = NULL;
if (*argument == NULL)
return NULL;
if (**argument == '[') {
nest++;
(*argument)++;
}
word = *argument;
word = last_ch = *argument;
while (**argument != '\0') {
if (nest && **argument == ']') {
**argument = '\0';
(*argument)++;
nest--;
continue;
}
if (nest == 0 && **argument == separator) {
**argument = '\0';
(*argument)++;
break;
}
if (**argument == '[') {
nest++;
} else if (nest && **argument == ']') {
nest--;
if (!first_close && nest == 0)
first_close = *argument;
}
last_ch = *argument;
(*argument)++;
}
/* If the word is surrounded with the nesting symbols [], strip them so we return
* the inner content only.
* If there were nesting symbols but embracing only part of the inner content, don't
* remove them. Example:
* Remove [] in get_word("[fc08::1]:other_token", ":")
* Don't remove [] in get_word("ip6=[fc08::1]:other_token", ":")
*/
if (*word == '[' && *last_ch == ']' && last_ch == first_close) {
word++;
*last_ch = '\0';
}
return *word ? word : NULL;
}
@@ -906,13 +917,24 @@ reader_parse_controller(Reader *reader,
opts = get_word(&argument, ':');
while (opts && *opts) {
gs_free_error GError *error = NULL;
char *opt;
char *tmp;
const char *opt_name;
char *opt;
const char *opt_value;
nm_auto_unref_ptrarray GPtrArray *opt_values = g_ptr_array_new();
gs_free char *opt_normalized = NULL;
opt_name = get_word(&opts, '=');
opt = get_word(&opts, ',');
opt_name = get_word(&opt, '=');
if (!_nm_setting_bond_validate_option(opt_name, opt, &error)) {
/* Normalize: convert ';' to ',' and remove '[]' from IPv6 addresses */
tmp = opt;
while ((opt_value = get_word(&tmp, ';')))
g_ptr_array_add(opt_values, (gpointer) opt_value);
g_ptr_array_add(opt_values, NULL);
opt_normalized = g_strjoinv(",", (char **) opt_values->pdata);
if (!_nm_setting_bond_validate_option(opt_name, opt_normalized, &error)) {
_LOGW(LOGD_CORE,
"Ignoring invalid bond option: %s%s%s = %s%s%s: %s",
NM_PRINT_FMT_QUOTE_STRING(opt_name),
@@ -920,7 +942,7 @@ reader_parse_controller(Reader *reader,
error->message);
continue;
}
nm_setting_bond_add_option(s_bond, opt_name, opt);
nm_setting_bond_add_option(s_bond, opt_name, opt_normalized);
}
mtu = get_word(&argument, ':');

View File

@@ -975,8 +975,8 @@ static void
test_bond(void)
{
gs_unref_hashtable GHashTable *connections = NULL;
const char *const *ARGV = NM_MAKE_STRV("rd.route=192.0.2.53::bong0",
"bond=bong0:eth0,eth1:mode=balance-rr:9000",
const char *const *ARGV = NM_MAKE_STRV("rd.route=192.0.2.53::bond0",
"bond=bond0:eth0,eth1:mode=balance-rr:9000",
"nameserver=203.0.113.53");
NMConnection *connection;
NMSettingConnection *s_con;
@@ -990,12 +990,12 @@ test_bond(void)
connections = _parse_cons(ARGV);
g_assert_cmpint(g_hash_table_size(connections), ==, 3);
connection = g_hash_table_lookup(connections, "bong0");
connection = g_hash_table_lookup(connections, "bond0");
nmtst_assert_connection_verifies_without_normalization(connection);
g_assert_cmpstr(nm_connection_get_connection_type(connection),
==,
NM_SETTING_BOND_SETTING_NAME);
g_assert_cmpstr(nm_connection_get_id(connection), ==, "bong0");
g_assert_cmpstr(nm_connection_get_id(connection), ==, "bond0");
controller_uuid = nm_connection_get_uuid(connection);
g_assert(controller_uuid);
@@ -1162,6 +1162,118 @@ test_bond_ip(void)
NM_CONNECTION_MULTI_CONNECT_SINGLE);
}
static void
test_bond_ip6_option(void)
{
/* Test that IPv6 addresses within [] are parsed fine in different positions */
gs_unref_hashtable GHashTable *connections = NULL;
const char *const *ARGV =
NM_MAKE_STRV("bond=bond0:eth0,eth1:arp_interval=100,ns_ip6_target=[fc08::1]",
"bond=bond1:eth2,eth3:arp_interval=100,ns_ip6_target=[fc08::1]:9000",
"bond=bond2:eth4,eth5:ns_ip6_target=[fc08::1],arp_interval=100");
NMConnection *connection;
NMSettingBond *s_bond;
connections = _parse_cons(ARGV);
g_assert_cmpint(g_hash_table_size(connections), ==, 9);
connection = g_hash_table_lookup(connections, "bond0");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"), ==, "fc08::1");
connection = g_hash_table_lookup(connections, "bond1");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"), ==, "fc08::1");
connection = g_hash_table_lookup(connections, "bond2");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"), ==, "fc08::1");
}
static void
test_bond_multi_values_option(void)
{
/* Test that semicolon-separated multi-valued options are parsed fine in different positions */
gs_unref_hashtable GHashTable *connections = NULL;
const char *const *ARGV =
NM_MAKE_STRV("bond=bond0:eth0,eth1:arp_interval=100,ns_ip6_target=[fc08::1];[fc08::2]",
"bond=bond1:eth2,eth3:arp_interval=100,ns_ip6_target=[fc08::1];[fc08::2]:9000",
"bond=bond2:eth4,eth5:ns_ip6_target=[fc08::1];[fc08::2],arp_interval=100",
"bond=bond3:eth6,eth7:arp_interval=100,arp_ip_target=10.0.0.1;10.0.0.2",
"bond=bond4:eth8,eth9:arp_interval=100,arp_ip_target=10.0.0.1;10.0.0.2:9000",
"bond=bond5:eth10,eth11:arp_ip_target=10.0.0.1;10.0.0.2,arp_interval=100");
NMConnection *connection;
NMSettingBond *s_bond;
connections = _parse_cons(ARGV);
g_assert_cmpint(g_hash_table_size(connections), ==, 18);
connection = g_hash_table_lookup(connections, "bond0");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"),
==,
"fc08::1,fc08::2");
connection = g_hash_table_lookup(connections, "bond1");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"),
==,
"fc08::1,fc08::2");
connection = g_hash_table_lookup(connections, "bond2");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "ns_ip6_target"),
==,
"fc08::1,fc08::2");
connection = g_hash_table_lookup(connections, "bond3");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "arp_ip_target"),
==,
"10.0.0.1,10.0.0.2");
connection = g_hash_table_lookup(connections, "bond4");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "arp_ip_target"),
==,
"10.0.0.1,10.0.0.2");
connection = g_hash_table_lookup(connections, "bond5");
nmtst_assert_connection_verifies_without_normalization(connection);
s_bond = nm_connection_get_setting_bond(connection);
g_assert(s_bond);
g_assert_cmpint(nm_setting_bond_get_num_options(s_bond), ==, 3);
g_assert_cmpstr(nm_setting_bond_get_option_by_name(s_bond, "arp_ip_target"),
==,
"10.0.0.1,10.0.0.2");
}
static void
test_bond_default(void)
{
@@ -2701,6 +2813,8 @@ main(int argc, char **argv)
g_test_add_func("/initrd/cmdline/bootdev", test_bootdev);
g_test_add_func("/initrd/cmdline/bond", test_bond);
g_test_add_func("/initrd/cmdline/bond/ip", test_bond_ip);
g_test_add_func("/initrd/cmdline/bond/ip6-option", test_bond_ip6_option);
g_test_add_func("/initrd/cmdline/bond/multi-values-option", test_bond_multi_values_option);
g_test_add_func("/initrd/cmdline/bond/default", test_bond_default);
g_test_add_func("/initrd/cmdline/team", test_team);
g_test_add_func("/initrd/cmdline/vlan", test_vlan);