Re: [tipc-discussion] [net-next 3/4] tipc: refactor function tipc_sk_anc_data_recv()
> -Original Message-
> From: jma...@redhat.com
> Sent: Thursday, March 25, 2021 10:56 PM
> To: tipc-discussion@lists.sourceforge.net
> Cc: Tung Quang Nguyen ; Hoang Huu Le
> ; Tuong Tong Lien
> ; jma...@redhat.com; ma...@donjonn.com;
> l...@redhat.com; ying@windriver.com;
> parthasarathy.bhuvara...@gmail.com
> Subject: [net-next 3/4] tipc: refactor function tipc_sk_anc_data_recv()
>
> From: Jon Maloy
>
> We refactor tipc_sk_anc_data_recv() to make it slighltly more
> comprehensible, but also to facilitate application of some additions
> to the code in a future commit.
>
> Signed-off-by: Jon Maloy
> ---
> net/tipc/socket.c | 85 +--
> 1 file changed, 38 insertions(+), 47 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 12a97755bc80..358d1f2494a7 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1730,67 +1730,58 @@ static void tipc_sk_set_orig_addr(struct msghdr *m,
> struct sk_buff *skb)
> static int tipc_sk_anc_data_recv(struct msghdr *m, struct sk_buff *skb,
>struct tipc_sock *tsk)
> {
> - struct tipc_msg *msg;
> - u32 anc_data[3];
> - u32 err;
> - u32 dest_type;
> - int has_name;
> - int res;
> + struct tipc_msg *hdr;
> + bool has_addr;
> + int data[12];
Potential memory leak here!
> + int dlen, rc;
>
> if (likely(m->msg_controllen == 0))
> return 0;
> - msg = buf_msg(skb);
>
> - /* Optionally capture errored message object(s) */
> - err = msg ? msg_errcode(msg) : 0;
> - if (unlikely(err)) {
> - anc_data[0] = err;
> - anc_data[1] = msg_data_sz(msg);
> - res = put_cmsg(m, SOL_TIPC, TIPC_ERRINFO, 8, anc_data);
> - if (res)
> - return res;
> - if (anc_data[1]) {
> - if (skb_linearize(skb))
> - return -ENOMEM;
> - msg = buf_msg(skb);
> - res = put_cmsg(m, SOL_TIPC, TIPC_RETDATA, anc_data[1],
> -msg_data(msg));
> - if (res)
> - return res;
> - }
> + hdr = buf_msg(skb);
> + dlen = msg_data_sz(hdr);
> +
> + /* Capture errored message object, if any */
> + if (msg_errcode(hdr)) {
> + if (skb_linearize(skb))
> + return -ENOMEM;
> + hdr = buf_msg(skb);
> + data[0] = msg_errcode(hdr);
> + data[1] = dlen;
> + rc = put_cmsg(m, SOL_TIPC, TIPC_ERRINFO, 8, data);
> + if (rc || !dlen)
> + return rc;
> + rc = put_cmsg(m, SOL_TIPC, TIPC_RETDATA, dlen, msg_data(hdr));
> + if (rc)
> + return rc;
> }
>
> - /* Optionally capture message destination object */
> - dest_type = msg ? msg_type(msg) : TIPC_DIRECT_MSG;
> - switch (dest_type) {
> + /* Capture TIPC_SERVICE_ADDR/RANGE destination address, if any */
> + switch (msg_type(hdr)) {
> case TIPC_NAMED_MSG:
> - has_name = 1;
> - anc_data[0] = msg_nametype(msg);
> - anc_data[1] = msg_namelower(msg);
> - anc_data[2] = msg_namelower(msg);
> + has_addr = true;
> + data[0] = msg_nametype(hdr);
> + data[1] = msg_namelower(hdr);
> + data[2] = data[1];
> break;
> case TIPC_MCAST_MSG:
> - has_name = 1;
> - anc_data[0] = msg_nametype(msg);
> - anc_data[1] = msg_namelower(msg);
> - anc_data[2] = msg_nameupper(msg);
> + has_addr = true;
> + data[0] = msg_nametype(hdr);
> + data[1] = msg_namelower(hdr);
> + data[2] = msg_nameupper(hdr);
> break;
> case TIPC_CONN_MSG:
> - has_name = !!tsk->conn_addrtype;
> - anc_data[0] = msg_nametype(>phdr);
> - anc_data[1] = msg_nameinst(>phdr);
> - anc_data[2] = anc_data[1];
> + has_addr = !!tsk->conn_addrtype;
> + data[0] = msg_nametype(>phdr);
> + data[1] = msg_nameinst(>phdr);
> + data[2] = data[1];
> break;
> default:
> - has_name = 0;
> - }
> - if (has_name) {
> - res = put_cmsg(m, SOL_TIPC, TIPC_DESTNAME, 12, anc_data);
> - if (res)
> - return res;
> + has_addr = false;
> }
> -
> - return 0;
> + if (!has_addr)
> + return 0;
> + return put_cmsg(m, SOL_TIPC, TIPC_DESTNAME, 12, data);
> }
>
> static struct sk_buff *tipc_sk_build_ack(struct tipc_sock *tsk)
> --
> 2.29.2
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
[tipc-discussion] [iproute2-next] tipc: use the libmnl functions in lib/mnl_utils.c
To avoid code duplication, tipc should be converted to use the helper
functions for working with libmnl in lib/mnl_utils.c
Acked-by: Jon Maloy
Signed-off-by: Hoang Le
---
tipc/bearer.c| 38 ++
tipc/cmdl.c | 2 -
tipc/link.c | 37 +
tipc/media.c | 15 +++---
tipc/msg.c | 132 +++
tipc/msg.h | 2 +-
tipc/nametable.c | 5 +-
tipc/node.c | 33 +---
tipc/peer.c | 8 ++-
tipc/socket.c| 10 ++--
tipc/tipc.c | 21 +++-
11 files changed, 83 insertions(+), 220 deletions(-)
diff --git a/tipc/bearer.c b/tipc/bearer.c
index 4470819e4a96..2afc48b9b108 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -21,9 +21,6 @@
#include
#include
-#include
-#include
-
#include "utils.h"
#include "cmdl.h"
#include "msg.h"
@@ -101,11 +98,11 @@ static int get_netid_cb(const struct nlmsghdr *nlh, void
*data)
static int generate_multicast(short af, char *buf, int bufsize)
{
- int netid;
- char mnl_msg[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh;
+ int netid;
- if (!(nlh = msg_init(mnl_msg, TIPC_NL_NET_GET))) {
+ nlh = msg_init(TIPC_NL_NET_GET);
+ if (!nlh) {
fprintf(stderr, "error, message initialization failed\n");
return -1;
}
@@ -399,7 +396,6 @@ static int cmd_bearer_add_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
char *media;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt *opt;
struct nlattr *attrs;
struct opt opts[] = {
@@ -435,7 +431,8 @@ static int cmd_bearer_add_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_ADD))) {
+ nlh = msg_init(TIPC_NL_BEARER_ADD);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -482,7 +479,6 @@ static int cmd_bearer_enable(struct nlmsghdr *nlh, const
struct cmd *cmd,
int err;
struct opt *opt;
struct nlattr *nest;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt opts[] = {
{ "device", OPT_KEYVAL, NULL },
{ "domain", OPT_KEYVAL, NULL },
@@ -508,7 +504,8 @@ static int cmd_bearer_enable(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_ENABLE))) {
+ nlh = msg_init(TIPC_NL_BEARER_ENABLE);
+ if (!nlh) {
fprintf(stderr, "error: message initialisation failed\n");
return -1;
}
@@ -563,7 +560,6 @@ static int cmd_bearer_disable(struct nlmsghdr *nlh, const
struct cmd *cmd,
struct cmdl *cmdl, void *data)
{
int err;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlattr *nest;
struct opt opts[] = {
{ "device", OPT_KEYVAL, NULL },
@@ -584,7 +580,8 @@ static int cmd_bearer_disable(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_DISABLE))) {
+ nlh = msg_init(TIPC_NL_BEARER_DISABLE);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -628,7 +625,6 @@ static int cmd_bearer_set_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
int err;
int val;
int prop;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlattr *props;
struct nlattr *attrs;
struct opt opts[] = {
@@ -675,7 +671,8 @@ static int cmd_bearer_set_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
}
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_SET))) {
+ nlh = msg_init(TIPC_NL_BEARER_SET);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -876,7 +873,6 @@ static int cmd_bearer_get_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
char *media;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt *opt;
struct cb_data cb_data = {0};
struct nlattr *attrs;
@@ -918,7 +914,8 @@ static int cmd_bearer_get_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_GET))) {
+ nlh = msg_init(TIPC_NL_BEARER_GET);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -956,7 +953,6 @@ static int cmd_bearer_get_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
int prop;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlattr *attrs;
struct opt
[tipc-discussion] [net] tipc: fix unique bearer names sanity check
When enabling a bearer by name, we don't sanity check its name with
higher slot in bearer list. This may have the effect that the name
of an already enabled bearer bypasses the check.
To fix the above issue, we just perform an extra checking with all
existing bearers.
Fixes: cb30a63384bc9 ("tipc: refactor function tipc_enable_bearer()")
Cc: sta...@vger.kernel.org
Acked-by: Jon Maloy
Signed-off-by: Hoang Le
---
net/tipc/bearer.c | 46 +++---
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index d47e0b940ac9..443f8e5b9477 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -256,6 +256,7 @@ static int tipc_enable_bearer(struct net *net, const char
*name,
int bearer_id = 0;
int res = -EINVAL;
char *errstr = "";
+ u32 i;
if (!bearer_name_validate(name, _names)) {
errstr = "illegal name";
@@ -280,31 +281,38 @@ static int tipc_enable_bearer(struct net *net, const char
*name,
prio = m->priority;
/* Check new bearer vs existing ones and find free bearer id if any */
- while (bearer_id < MAX_BEARERS) {
- b = rtnl_dereference(tn->bearer_list[bearer_id]);
- if (!b)
- break;
+ bearer_id = MAX_BEARERS;
+ i = MAX_BEARERS;
+ while (i-- != 0) {
+ b = rtnl_dereference(tn->bearer_list[i]);
+ if (!b) {
+ bearer_id = i;
+ continue;
+ }
if (!strcmp(name, b->name)) {
errstr = "already enabled";
NL_SET_ERR_MSG(extack, "Already enabled");
goto rejected;
}
- bearer_id++;
- if (b->priority != prio)
- continue;
- if (++with_this_prio <= 2)
- continue;
- pr_warn("Bearer <%s>: already 2 bearers with priority %u\n",
- name, prio);
- if (prio == TIPC_MIN_LINK_PRI) {
- errstr = "cannot adjust to lower";
- NL_SET_ERR_MSG(extack, "Cannot adjust to lower");
- goto rejected;
+
+ if (b->priority == prio &&
+ (++with_this_prio > 2)) {
+ pr_warn("Bearer <%s>: already 2 bearers with priority
%u\n",
+ name, prio);
+
+ if (prio == TIPC_MIN_LINK_PRI) {
+ errstr = "cannot adjust to lower";
+ NL_SET_ERR_MSG(extack, "Cannot adjust to
lower");
+ goto rejected;
+ }
+
+ pr_warn("Bearer <%s>: trying with adjusted priority\n",
+ name);
+ prio--;
+ bearer_id = MAX_BEARERS;
+ i = MAX_BEARERS;
+ with_this_prio = 1;
}
- pr_warn("Bearer <%s>: trying with adjusted priority\n", name);
- prio--;
- bearer_id = 0;
- with_this_prio = 1;
}
if (bearer_id >= MAX_BEARERS) {
--
2.25.1
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [iproute2-next] tipc: use the libmnl functions in lib/mnl_utils.c
On 3/29/21 6:59 AM, Hoang Le wrote:
To avoid duplication,
avoid code duplication
tipc should be converted to use the helper
functions for working with libmnl in lib/mnl_utils.c
Signed-off-by: Hoang Le
---
tipc/bearer.c| 38 ++
tipc/cmdl.c | 2 -
tipc/link.c | 37 +
tipc/media.c | 15 +++---
tipc/msg.c | 132 +++
tipc/msg.h | 2 +-
tipc/nametable.c | 5 +-
tipc/node.c | 33 +---
tipc/peer.c | 8 ++-
tipc/socket.c| 10 ++--
tipc/tipc.c | 21 +++-
11 files changed, 83 insertions(+), 220 deletions(-)
diff --git a/tipc/bearer.c b/tipc/bearer.c
index 4470819e4a96..2afc48b9b108 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -21,9 +21,6 @@
#include
#include
-#include
-#include
-
#include "utils.h"
#include "cmdl.h"
#include "msg.h"
@@ -101,11 +98,11 @@ static int get_netid_cb(const struct nlmsghdr *nlh, void
*data)
static int generate_multicast(short af, char *buf, int bufsize)
{
- int netid;
- char mnl_msg[MNL_SOCKET_BUFFER_SIZE];
struct nlmsghdr *nlh;
+ int netid;
- if (!(nlh = msg_init(mnl_msg, TIPC_NL_NET_GET))) {
+ nlh = msg_init(TIPC_NL_NET_GET);
+ if (!nlh) {
fprintf(stderr, "error, message initialization failed\n");
return -1;
}
@@ -399,7 +396,6 @@ static int cmd_bearer_add_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
char *media;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt *opt;
struct nlattr *attrs;
struct opt opts[] = {
@@ -435,7 +431,8 @@ static int cmd_bearer_add_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_ADD))) {
+ nlh = msg_init(TIPC_NL_BEARER_ADD);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -482,7 +479,6 @@ static int cmd_bearer_enable(struct nlmsghdr *nlh, const
struct cmd *cmd,
int err;
struct opt *opt;
struct nlattr *nest;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt opts[] = {
{ "device", OPT_KEYVAL, NULL },
{ "domain", OPT_KEYVAL, NULL },
@@ -508,7 +504,8 @@ static int cmd_bearer_enable(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_ENABLE))) {
+ nlh = msg_init(TIPC_NL_BEARER_ENABLE);
+ if (!nlh) {
fprintf(stderr, "error: message initialisation failed\n");
return -1;
}
@@ -563,7 +560,6 @@ static int cmd_bearer_disable(struct nlmsghdr *nlh, const
struct cmd *cmd,
struct cmdl *cmdl, void *data)
{
int err;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlattr *nest;
struct opt opts[] = {
{ "device", OPT_KEYVAL, NULL },
@@ -584,7 +580,8 @@ static int cmd_bearer_disable(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_DISABLE))) {
+ nlh = msg_init(TIPC_NL_BEARER_DISABLE);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -628,7 +625,6 @@ static int cmd_bearer_set_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
int err;
int val;
int prop;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct nlattr *props;
struct nlattr *attrs;
struct opt opts[] = {
@@ -675,7 +671,8 @@ static int cmd_bearer_set_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
}
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_SET))) {
+ nlh = msg_init(TIPC_NL_BEARER_SET);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -876,7 +873,6 @@ static int cmd_bearer_get_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
char *media;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct opt *opt;
struct cb_data cb_data = {0};
struct nlattr *attrs;
@@ -918,7 +914,8 @@ static int cmd_bearer_get_media(struct nlmsghdr *nlh, const
struct cmd *cmd,
return -EINVAL;
}
- if (!(nlh = msg_init(buf, TIPC_NL_BEARER_GET))) {
+ nlh = msg_init(TIPC_NL_BEARER_GET);
+ if (!nlh) {
fprintf(stderr, "error, message initialisation failed\n");
return -1;
}
@@ -956,7 +953,6 @@ static int cmd_bearer_get_prop(struct nlmsghdr *nlh, const
struct cmd *cmd,
{
int err;
int prop;
- char buf[MNL_SOCKET_BUFFER_SIZE];
struct
Re: [tipc-discussion] [net v2] tipc: fix unique bearer names sanity check
On 3/29/21 10:16 PM, Hoang Le wrote:
When enabling a bearer
s/with identify//g
by name, we don't sanity check
its name with higher slot in bearer list. This lead to duplicate
bearer names bypassed the check.
This may have the effect that the name of an already enabled bearer
bypasses the check.
To fix the above issue, we just perform an extra checking with all
existing bearers.
Fixes: cb30a63384bc9 ("tipc: refactor function tipc_enable_bearer()")
Signed-off-by: Hoang Le
---
net/tipc/bearer.c | 45 ++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index d47e0b940ac9..94eddc67d52e 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -256,6 +256,7 @@ static int tipc_enable_bearer(struct net *net, const char
*name,
int bearer_id = 0;
int res = -EINVAL;
char *errstr = "";
+ u32 i;
if (!bearer_name_validate(name, _names)) {
errstr = "illegal name";
@@ -280,31 +281,37 @@ static int tipc_enable_bearer(struct net *net, const char
*name,
prio = m->priority;
/* Check new bearer vs existing ones and find free bearer id if any */
- while (bearer_id < MAX_BEARERS) {
- b = rtnl_dereference(tn->bearer_list[bearer_id]);
- if (!b)
- break;
+ bearer_id = MAX_BEARERS;
+ i = MAX_BEARERS;
+ while (i-- != 0) {
+ b = rtnl_dereference(tn->bearer_list[i]);
+ if (!b) {
+ bearer_id = i;
+ continue;
+ }
if (!strcmp(name, b->name)) {
errstr = "already enabled";
NL_SET_ERR_MSG(extack, "Already enabled");
goto rejected;
}
- bearer_id++;
- if (b->priority != prio)
- continue;
- if (++with_this_prio <= 2)
- continue;
- pr_warn("Bearer <%s>: already 2 bearers with priority %u\n",
- name, prio);
- if (prio == TIPC_MIN_LINK_PRI) {
- errstr = "cannot adjust to lower";
- NL_SET_ERR_MSG(extack, "Cannot adjust to lower");
- goto rejected;
+
+ if (b->priority == prio &&
+ (++with_this_prio > 2)) {
+ pr_warn("Bearer <%s>: already 2 bearers with priority
%u\n",
+ name, prio);
+
+ if (prio == TIPC_MIN_LINK_PRI) {
+ errstr = "cannot adjust to lower";
+ NL_SET_ERR_MSG(extack, "Cannot adjust to
lower");
+ goto rejected;
+ }
+
+ pr_warn("Bearer <%s>: trying with adjusted priority\n",
name);
+ prio--;
+ bearer_id = MAX_BEARERS;
+ i = MAX_BEARERS;
+ with_this_prio = 1;
}
- pr_warn("Bearer <%s>: trying with adjusted priority\n", name);
- prio--;
- bearer_id = 0;
- with_this_prio = 1;
}
if (bearer_id >= MAX_BEARERS) {
Acked-by: Jon Maloy
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
