* [PATCH net 0/3] tools: ynl: fix header guards and impossible errors
@ 2024-02-17 0:17 Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 1/3] tools: ynl: fix header guards Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:17 UTC (permalink / raw
To: davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, nicolas.dichtel,
willemb, Jakub Kicinski
Fix header guards for new families. I must have installed the latest
headers on my system when I was developing. Now it seems system update
overwrote them back to something slightly older, revealing the guards
I put in are wrong.
Also fix bugs discovered while I was hacking in low level stuff in YNL
and kept breaking the socket, really exercising the "impossible" paths.
Jakub Kicinski (3):
tools: ynl: fix header guards
tools: ynl: make sure we always pass yarg to mnl_cb_run
tools: ynl: don't leak mcast_groups on init error
tools/net/ynl/Makefile.deps | 4 ++--
tools/net/ynl/lib/ynl.c | 19 +++++++++++++++----
2 files changed, 17 insertions(+), 6 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/3] tools: ynl: fix header guards
2024-02-17 0:17 [PATCH net 0/3] tools: ynl: fix header guards and impossible errors Jakub Kicinski
@ 2024-02-17 0:17 ` Jakub Kicinski
2024-02-19 17:18 ` Nicolas Dichtel
2024-02-17 0:17 ` [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error Jakub Kicinski
2 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:17 UTC (permalink / raw
To: davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, nicolas.dichtel,
willemb, Jakub Kicinski
devlink and ethtool have a trailing _ in the header guard. I must have
copy/pasted it into new guards, assuming it's a headers_install artifact.
Fixes: 8f109e91b852 ("tools: ynl: include dpll and mptcp_pm in C codegen")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: chuck.lever@oracle.com
CC: jiri@resnulli.us
---
tools/net/ynl/Makefile.deps | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps
index e6154a1255f8..7dcec16da509 100644
--- a/tools/net/ynl/Makefile.deps
+++ b/tools/net/ynl/Makefile.deps
@@ -15,9 +15,9 @@ UAPI_PATH:=../../../../include/uapi/
get_hdr_inc=-D$(1) -include $(UAPI_PATH)/linux/$(2)
CFLAGS_devlink:=$(call get_hdr_inc,_LINUX_DEVLINK_H_,devlink.h)
-CFLAGS_dpll:=$(call get_hdr_inc,_LINUX_DPLL_H_,dpll.h)
+CFLAGS_dpll:=$(call get_hdr_inc,_LINUX_DPLL_H,dpll.h)
CFLAGS_ethtool:=$(call get_hdr_inc,_LINUX_ETHTOOL_NETLINK_H_,ethtool_netlink.h)
CFLAGS_handshake:=$(call get_hdr_inc,_LINUX_HANDSHAKE_H,handshake.h)
-CFLAGS_mptcp_pm:=$(call get_hdr_inc,_LINUX_MPTCP_PM_H_,mptcp_pm.h)
+CFLAGS_mptcp_pm:=$(call get_hdr_inc,_LINUX_MPTCP_PM_H,mptcp_pm.h)
CFLAGS_netdev:=$(call get_hdr_inc,_LINUX_NETDEV_H,netdev.h)
CFLAGS_nfsd:=$(call get_hdr_inc,_LINUX_NFSD_NETLINK_H,nfsd_netlink.h)
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run
2024-02-17 0:17 [PATCH net 0/3] tools: ynl: fix header guards and impossible errors Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 1/3] tools: ynl: fix header guards Jakub Kicinski
@ 2024-02-17 0:17 ` Jakub Kicinski
2024-02-19 17:18 ` Nicolas Dichtel
2024-02-17 0:17 ` [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error Jakub Kicinski
2 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:17 UTC (permalink / raw
To: davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, nicolas.dichtel,
willemb, Jakub Kicinski
There is one common error handler in ynl - ynl_cb_error().
It expects priv to be a pointer to struct ynl_parse_arg AKA yarg.
To avoid potential crashes if we encounter a stray NLMSG_ERROR
always pass yarg as priv (or a struct which has it as the first
member).
ynl_cb_null() has a similar problem directly - it expects yarg
but priv passed by the caller is ys.
Found by code inspection.
Fixes: 86878f14d71a ("tools: ynl: user space helpers")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Further cleanup to enforce the types in net-next..
---
CC: nicolas.dichtel@6wind.com
CC: willemb@google.com
---
tools/net/ynl/lib/ynl.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
index c82a7f41b31c..9e41c8c0cc99 100644
--- a/tools/net/ynl/lib/ynl.c
+++ b/tools/net/ynl/lib/ynl.c
@@ -466,6 +466,8 @@ ynl_gemsg_start_dump(struct ynl_sock *ys, __u32 id, __u8 cmd, __u8 version)
int ynl_recv_ack(struct ynl_sock *ys, int ret)
{
+ struct ynl_parse_arg yarg = { .ys = ys, };
+
if (!ret) {
yerr(ys, YNL_ERROR_EXPECT_ACK,
"Expecting an ACK but nothing received");
@@ -478,7 +480,7 @@ int ynl_recv_ack(struct ynl_sock *ys, int ret)
return ret;
}
return mnl_cb_run(ys->rx_buf, ret, ys->seq, ys->portid,
- ynl_cb_null, ys);
+ ynl_cb_null, &yarg);
}
int ynl_cb_null(const struct nlmsghdr *nlh, void *data)
@@ -741,11 +743,14 @@ static int ynl_ntf_parse(struct ynl_sock *ys, const struct nlmsghdr *nlh)
static int ynl_ntf_trampoline(const struct nlmsghdr *nlh, void *data)
{
- return ynl_ntf_parse((struct ynl_sock *)data, nlh);
+ struct ynl_parse_arg *yarg = data;
+
+ return ynl_ntf_parse(yarg->ys, nlh);
}
int ynl_ntf_check(struct ynl_sock *ys)
{
+ struct ynl_parse_arg yarg = { .ys = ys, };
ssize_t len;
int err;
@@ -767,7 +772,7 @@ int ynl_ntf_check(struct ynl_sock *ys)
return len;
err = mnl_cb_run2(ys->rx_buf, len, ys->seq, ys->portid,
- ynl_ntf_trampoline, ys,
+ ynl_ntf_trampoline, &yarg,
ynl_cb_array, NLMSG_MIN_TYPE);
if (err < 0)
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error
2024-02-17 0:17 [PATCH net 0/3] tools: ynl: fix header guards and impossible errors Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 1/3] tools: ynl: fix header guards Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run Jakub Kicinski
@ 2024-02-17 0:17 ` Jakub Kicinski
2024-02-19 17:19 ` Nicolas Dichtel
2 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2024-02-17 0:17 UTC (permalink / raw
To: davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, nicolas.dichtel,
willemb, Jakub Kicinski
Make sure to free the already-parsed mcast_groups if
we don't get an ack from the kernel when reading family info.
This is part of the ynl_sock_create() error path, so we won't
get a call to ynl_sock_destroy() to free them later.
Fixes: 86878f14d71a ("tools: ynl: user space helpers")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: nicolas.dichtel@6wind.com
CC: willemb@google.com
---
tools/net/ynl/lib/ynl.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
index 9e41c8c0cc99..6e6d474c8366 100644
--- a/tools/net/ynl/lib/ynl.c
+++ b/tools/net/ynl/lib/ynl.c
@@ -588,7 +588,13 @@ static int ynl_sock_read_family(struct ynl_sock *ys, const char *family_name)
return err;
}
- return ynl_recv_ack(ys, err);
+ err = ynl_recv_ack(ys, err);
+ if (err < 0) {
+ free(ys->mcast_groups);
+ return err;
+ }
+
+ return 0;
}
struct ynl_sock *
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 1/3] tools: ynl: fix header guards
2024-02-17 0:17 ` [PATCH net 1/3] tools: ynl: fix header guards Jakub Kicinski
@ 2024-02-19 17:18 ` Nicolas Dichtel
0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2024-02-19 17:18 UTC (permalink / raw
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, willemb
Le 17/02/2024 à 01:17, Jakub Kicinski a écrit :
> devlink and ethtool have a trailing _ in the header guard. I must have
> copy/pasted it into new guards, assuming it's a headers_install artifact.
>
> Fixes: 8f109e91b852 ("tools: ynl: include dpll and mptcp_pm in C codegen")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run
2024-02-17 0:17 ` [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run Jakub Kicinski
@ 2024-02-19 17:18 ` Nicolas Dichtel
0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2024-02-19 17:18 UTC (permalink / raw
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, willemb
Le 17/02/2024 à 01:17, Jakub Kicinski a écrit :
> There is one common error handler in ynl - ynl_cb_error().
> It expects priv to be a pointer to struct ynl_parse_arg AKA yarg.
> To avoid potential crashes if we encounter a stray NLMSG_ERROR
> always pass yarg as priv (or a struct which has it as the first
> member).
>
> ynl_cb_null() has a similar problem directly - it expects yarg
> but priv passed by the caller is ys.
>
> Found by code inspection.
>
> Fixes: 86878f14d71a ("tools: ynl: user space helpers")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error
2024-02-17 0:17 ` [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error Jakub Kicinski
@ 2024-02-19 17:19 ` Nicolas Dichtel
0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2024-02-19 17:19 UTC (permalink / raw
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, chuck.lever, jiri, willemb
Le 17/02/2024 à 01:17, Jakub Kicinski a écrit :
> Make sure to free the already-parsed mcast_groups if
> we don't get an ack from the kernel when reading family info.
> This is part of the ynl_sock_create() error path, so we won't
> get a call to ynl_sock_destroy() to free them later.
>
> Fixes: 86878f14d71a ("tools: ynl: user space helpers")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-19 17:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-17 0:17 [PATCH net 0/3] tools: ynl: fix header guards and impossible errors Jakub Kicinski
2024-02-17 0:17 ` [PATCH net 1/3] tools: ynl: fix header guards Jakub Kicinski
2024-02-19 17:18 ` Nicolas Dichtel
2024-02-17 0:17 ` [PATCH net 2/3] tools: ynl: make sure we always pass yarg to mnl_cb_run Jakub Kicinski
2024-02-19 17:18 ` Nicolas Dichtel
2024-02-17 0:17 ` [PATCH net 3/3] tools: ynl: don't leak mcast_groups on init error Jakub Kicinski
2024-02-19 17:19 ` Nicolas Dichtel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).