[PATCHv2] teamd: escape some sensitive characters in ifname with double quotation marks
by Xin Long
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to escape dot inside ifname by quoting ifname with double
quotation marks, which can also escape any other characters, like '['
and '\"' in ifname.
Note that this patch can also fix the issue that users set/get state
like team0 has two ports eth1.1 and eth2\" :
teamdctl team0 state item get 'ports."eth1.1".link_watches.up'
teamdctl team0 state item get 'ports."eth2"".link_watches.up'
No matter what character is inside ifname, users just need to quoting
it with double quotation marks if necessary.
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
man/teamdctl.8 | 2 ++
teamd/teamd_json.c | 11 ++++++++++-
teamd/teamd_state.c | 24 ++++++++++++++++++------
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/man/teamdctl.8 b/man/teamdctl.8
index 55262e6..fb2876d 100644
--- a/man/teamdctl.8
+++ b/man/teamdctl.8
@@ -50,6 +50,8 @@ Prints out state of teamd parsed from JSON state document.
.TP
.BI "state item get " state_item_path
Finds state item in JSON state document and returns its value.
+
+If PORTIFNAME in state_item_path has any sensitive character, use double quotation marks to escape it, like: ports."eth1.1".link_watches.up. To process state_item_path of 'state item set' is the same as here.
.TP
.BI "state item set " "state_item_path value"
Finds state item in JSON state document and sets its value by value parameter. This is available only for a limited number of paths:
diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
index 5fd5e8e..b5e5464 100644
--- a/teamd/teamd_json.c
+++ b/teamd/teamd_json.c
@@ -76,7 +76,16 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
char tmp = 0; /* gcc needs this initialized */
ptr++;
- end = __strchrs(ptr, ".[");
+ if (*ptr == '\"') {
+ ptr++;
+ end = strrchr(ptr, '\"');
+ if (end) {
+ *end = '\0';
+ end++;
+ }
+ } else {
+ end = __strchrs(ptr, ".[");
+ }
if (end) {
tmp = *end;
*end = '\0';
diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
index 51fac8d..ab64db9 100644
--- a/teamd/teamd_state.c
+++ b/teamd/teamd_state.c
@@ -195,7 +195,7 @@ static int teamd_state_build_val_json_subpath(json_t **p_vg_json_obj,
char *dot;
if (tdport)
- ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX "%s%s",
+ ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX "\"%s\"%s",
tdport->ifname, subpath);
else
ret = asprintf(&path, "$%s", subpath);
@@ -296,13 +296,25 @@ static int __find_by_item_path(struct teamd_state_val_item **p_item,
if (!strncmp(item_path, TEAMD_STATE_PER_PORT_PREFIX,
strlen(TEAMD_STATE_PER_PORT_PREFIX))) {
+ char *ifname_start, *ifname_end;
struct teamd_port *cur_tdport;
- char *ifname_start = strchr(item_path, '.') + 1;
- char *ifname_end = strchr(ifname_start, '.');
- size_t ifname_len = ifname_end - ifname_start;
+ size_t ifname_len;
+
+ ifname_start = strchr(item_path, '.') + 1;
+ if (*ifname_start == '\"') {
+ ifname_start++;
+ ifname_end = strrchr(ifname_start, '\"');
+ if (!ifname_end)
+ return -EINVAL;
+ ifname_len = ifname_end - ifname_start;
+ ifname_end++;
+ } else {
+ ifname_end = strchr(ifname_start, '.');
+ if (!ifname_end)
+ return -EINVAL;
+ ifname_len = ifname_end - ifname_start;
+ }
- if (!ifname_end)
- return -EINVAL;
subpath = ifname_end + 1;
teamd_for_each_tdport(cur_tdport, ctx) {
--
2.1.0
6 years, 9 months
[jpirko/libteam] b30381: teamd: escape some sensitive characters in
ifname ...
by GitHub
Branch: refs/heads/master
Home: https://github.com/jpirko/libteam
Commit: b30381795c691be9256c49616105d022a2884d1c
https://github.com/jpirko/libteam/commit/b30381795c691be9256c49616105d022...
Author: Xin Long <lucien.xin(a)gmail.com>
Date: 2016-12-26 (Mon, 26 Dec 2016)
Changed paths:
M man/teamdctl.8
M teamd/teamd_json.c
M teamd/teamd_state.c
Log Message:
-----------
teamd: escape some sensitive characters in ifname with double quotation marks
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to escape dot inside ifname by quoting ifname with double
quotation marks, which can also escape any other characters, like '['
and '\"' in ifname.
Note that this patch can also fix the issue that users set/get state
like team0 has two ports eth1.1 and eth2\" :
teamdctl team0 state item get 'ports."eth1.1".link_watches.up'
teamdctl team0 state item get 'ports."eth2"".link_watches.up'
No matter what character is inside ifname, users just need to quoting
it with double quotation marks if necessary.
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
Signed-off-by: Jiri Pirko <jiri(a)mellanox.com>
6 years, 9 months
Fwd: [PATCH] teamd: escape some specail characters inside ifname with
double quotes
by Xin Long
(add to libteam)
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to escape dot inside ifname by quoting ifname with
double quotes, which can also escape any other characters, like
'[' and '\"' in ifname.
Note that this patch can also fix the issue that users set/get state
like team0 has two ports eth1.1 and eth2\" :
teamdctl team0 state item get
'ports."eth1.1".link_watches.list.link_watch_0.name'
teamdctl team0 state item get
'ports."eth2"".link_watches.list.link_watch_0.name'
No matter what character is inside ifname, users just need to quoting
it with double quotes if necessary.
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
teamd/teamd_json.c | 11 ++++++++++-
teamd/teamd_state.c | 24 ++++++++++++++++++------
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
index 5fd5e8e..b5e5464 100644
--- a/teamd/teamd_json.c
+++ b/teamd/teamd_json.c
@@ -76,7 +76,16 @@ static int __teamd_json_path_lite_va(json_t
**p_json_obj, json_t *json_root,
char tmp = 0; /* gcc needs this initialized */
ptr++;
- end = __strchrs(ptr, ".[");
+ if (*ptr == '\"') {
+ ptr++;
+ end = strrchr(ptr, '\"');
+ if (end) {
+ *end = '\0';
+ end++;
+ }
+ } else {
+ end = __strchrs(ptr, ".[");
+ }
if (end) {
tmp = *end;
*end = '\0';
diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
index 51fac8d..ab64db9 100644
--- a/teamd/teamd_state.c
+++ b/teamd/teamd_state.c
@@ -195,7 +195,7 @@ static int
teamd_state_build_val_json_subpath(json_t **p_vg_json_obj,
char *dot;
if (tdport)
- ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX "%s%s",
+ ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX
"\"%s\"%s",
tdport->ifname, subpath);
else
ret = asprintf(&path, "$%s", subpath);
@@ -296,13 +296,25 @@ static int __find_by_item_path(struct
teamd_state_val_item **p_item,
if (!strncmp(item_path, TEAMD_STATE_PER_PORT_PREFIX,
strlen(TEAMD_STATE_PER_PORT_PREFIX))) {
+ char *ifname_start, *ifname_end;
struct teamd_port *cur_tdport;
- char *ifname_start = strchr(item_path, '.') + 1;
- char *ifname_end = strchr(ifname_start, '.');
- size_t ifname_len = ifname_end - ifname_start;
+ size_t ifname_len;
+
+ ifname_start = strchr(item_path, '.') + 1;
+ if (*ifname_start == '\"') {
+ ifname_start++;
+ ifname_end = strrchr(ifname_start, '\"');
+ if (!ifname_end)
+ return -EINVAL;
+ ifname_len = ifname_end - ifname_start;
+ ifname_end++;
+ } else {
+ ifname_end = strchr(ifname_start, '.');
+ if (!ifname_end)
+ return -EINVAL;
+ ifname_len = ifname_end - ifname_start;
+ }
- if (!ifname_end)
- return -EINVAL;
subpath = ifname_end + 1;
teamd_for_each_tdport(cur_tdport, ctx) {
--
2.1.0
6 years, 9 months
Re: [PATCHv2 net] team: team_port_add should check link_up before
enable port
by Xin Long
>>attachment is the scripts,
>>
>># ./setup.sh
>># ip netns exec ns1 ./team1.sh
>># ./team0.sh
>># ping 192.168.11.3
>>
>>the issue is team0 cannot switch to veth2, even if the veth0 has no carrier.
>>
>>> This should be definitelly fixed in user part.
>>now it can disable/enable it in lw_ethtool_event_watch_port_changed()
>>when receive event from kernel, but at the beginning, the first event from
>>adding port will not going to team_port_en_option_set, as new_link_up
>>== common_ppriv->link_up in lw_ethtool_event_watch_port_changed().
>>
>>maybe it should be fixed there ?
>
> Yes please.
Hi, Jiri,
considering this issue only happens when adding ports,
I found two places to fix it:
lw_ethtool_port_added() and link_watch_event_watch_port_added()
I prefer in lw_ethtool_port_added(), as only when using ethtool link_watch it
has this issue.
--- a/teamd/teamd_lw_ethtool.c
+++ b/teamd/teamd_lw_ethtool.c
@@ -156,6 +156,9 @@ static int lw_ethtool_port_added(struct teamd_context *ctx,
teamd_log_err("Failed to register event watch.");
goto delay_callback_del;
}
+ if (!team_is_port_link_up(tdport->team_port))
+ team_set_port_enabled(ctx->th, tdport->ifindex, false);
+
return 0;
what do you think ?
I also noticed another issue (not related to this one), at the first time
port_changed event is triggered, right after port_added event, there
is always a err log:
veth2: Failed to find "enabled" option.
It's from:
lw_ethtool_event_watch_port_changed -> ...
teamd_port_check_enable -> teamd_port_enabled
it's because at that time, no 'enabled' option is set in userspace yet.
do you think it's also a bug ?
6 years, 10 months
[PATCH] teamd: replace dot with space in ifname when dumping teamd state
by Xin Long
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to replace dot with space in ifname for then, after
being parsed, it would be changed back to dot.
Note that it's safe to change it with space, as space in ifname is
disallowed. Besides, no teamd state attributes/nodes include space
afaik, so no side effects could be caused by this fix.
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
teamd/teamd_json.c | 16 +++++++++++++++-
teamd/teamd_json.h | 1 +
teamd/teamd_state.c | 13 ++++++++++---
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
index 5fd5e8e..7082484 100644
--- a/teamd/teamd_json.c
+++ b/teamd/teamd_json.c
@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
return NULL;
}
+char *strchrchg(char *str, char s, char d)
+{
+ char *tmp = str;
+
+ while (*str != '\0') {
+ if (*str == s)
+ *str = d;
+ str++;
+ }
+
+ return tmp;
+}
+
#define TEAMD_JSON_PATH_MAXLEN 128
typedef json_t *(*obj_constructor_t)(void);
@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
*end = '\0';
}
prev_json_obj = json_obj;
- json_obj = json_object_get(prev_json_obj, ptr);
+ json_obj = json_object_get(prev_json_obj,
+ strchrchg(ptr, ' ', '.'));
if (!json_obj && obj_constructor) {
/* In case new object is not supposed to be
* leaf, use json_object() as a constructor.
diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
index 30e9eaa..3dffdb5 100644
--- a/teamd/teamd_json.h
+++ b/teamd/teamd_json.h
@@ -24,6 +24,7 @@
#define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII | JSON_SORT_KEYS)
+char *strchrchg(char *str, char s, char d);
int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
const char *fmt, va_list ap);
int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
index 51fac8d..d9931dc 100644
--- a/teamd/teamd_state.c
+++ b/teamd/teamd_state.c
@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t **p_vg_json_obj,
int err;
char *dot;
- if (tdport)
+ if (tdport) {
+ char *tmp;
+
+ if (asprintf(&tmp, tdport->ifname) == -1)
+ return -ENOMEM;
+
ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX "%s%s",
- tdport->ifname, subpath);
- else
+ strchrchg(tmp, '.', ' '), subpath);
+ free(tmp);
+ } else {
ret = asprintf(&path, "$%s", subpath);
+ }
if (ret == -1)
return -ENOMEM;
dot = strrchr(path, '.');
--
2.1.0
6 years, 10 months