1. Patchew
  2. MPTCP
  3. View series

[PATCH mptcp-next v2] mptcp: use GENL_REQ_ATTR_CHECK for token

Geliang Tang posted 1 patch 5 days, 9 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/a5dc0d188ebb30b3d541178a821b08f1124246ba.1734330271.git.tanggeliang@kylinos.cn
There is a newer version of this series
net/mptcp/pm.c           | 18 +++++++++---------
net/mptcp/pm_userspace.c |  5 +++--
2 files changed, 12 insertions(+), 11 deletions(-)
[PATCH mptcp-next v2] mptcp: use GENL_REQ_ATTR_CHECK for token
Posted by Geliang Tang 5 days, 9 hours ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

A more general way to check if MPTCP_PM_ATTR_TOKEN exists in 'info'
is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN) instead of
directly reading info->attrs[MPTCP_PM_ATTR_TOKEN] and then checking
if it's NULL.

So this patch uses GENL_REQ_ATTR_CHECK() for 'token' in 'info' in
mptcp_pm_get_addr(), mptcp_pm_dump_addr(), mptcp_pm_set_flags() and
mptcp_userspace_pm_get_sock().

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
v2:
 - use GENL_REQ_ATTR_CHECK in get_addr(), dump_addr() and set_flags()
   too.
---
 net/mptcp/pm.c           | 18 +++++++++---------
 net/mptcp/pm_userspace.c |  5 +++--
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 16c336c51940..7c8525ff4107 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -435,25 +435,25 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 
 int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
 {
-	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_get_addr(skb, info);
-	return mptcp_pm_nl_get_addr(skb, info);
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
+		return mptcp_pm_nl_get_addr(skb, info);
+	return mptcp_userspace_pm_get_addr(skb, info);
 }
 
 int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
 {
 	const struct genl_info *info = genl_info_dump(cb);
 
-	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_dump_addr(msg, cb);
-	return mptcp_pm_nl_dump_addr(msg, cb);
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
+		return mptcp_pm_nl_dump_addr(msg, cb);
+	return mptcp_userspace_pm_dump_addr(msg, cb);
 }
 
 int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
-	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_set_flags(skb, info);
-	return mptcp_pm_nl_set_flags(skb, info);
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
+		return mptcp_pm_nl_set_flags(skb, info);
+	return mptcp_userspace_pm_set_flags(skb, info);
 }
 
 void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 740a10d669f8..cc2b7c50d4d6 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -175,14 +175,15 @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
 
 static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
 {
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct mptcp_sock *msk;
+	struct nlattr *token;
 
-	if (!token) {
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN)) {
 		GENL_SET_ERR_MSG(info, "missing required token");
 		return NULL;
 	}
 
+	token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
 	if (!msk) {
 		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
-- 
2.45.2
Re: [PATCH mptcp-next v2] mptcp: use GENL_REQ_ATTR_CHECK for token
Posted by Matthieu Baerts 5 days, 8 hours ago 
Hi Geliang,

Thank you for this patch.

16 Dec 2024 07:25:43 Geliang Tang <geliang@kernel.org>:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> A more general way to check if MPTCP_PM_ATTR_TOKEN exists in 'info'
> is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN) instead of
> directly reading info->attrs[MPTCP_PM_ATTR_TOKEN] and then checking
> if it's NULL.
>
> So this patch uses GENL_REQ_ATTR_CHECK() for 'token' in 'info' in
> mptcp_pm_get_addr(), mptcp_pm_dump_addr(), mptcp_pm_set_flags() and
> mptcp_userspace_pm_get_sock().
>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> v2:
> - use GENL_REQ_ATTR_CHECK in get_addr(), dump_addr() and set_flags()
>    too.
> ---
> net/mptcp/pm.c           | 18 +++++++++---------
> net/mptcp/pm_userspace.c |  5 +++--
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 16c336c51940..7c8525ff4107 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -435,25 +435,25 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
>
> int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
> {
> -   if (info->attrs[MPTCP_PM_ATTR_TOKEN])
> -       return mptcp_userspace_pm_get_addr(skb, info);
> -   return mptcp_pm_nl_get_addr(skb, info);
> +   if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))

If I'm not mistaken, this will set an error if the attribute is not defined.

We should not use it here not to set errors when it is normal not to
set the token.

Same below.

> +       return mptcp_pm_nl_get_addr(skb, info);
> +   return mptcp_userspace_pm_get_addr(skb, info);
> }
>
> int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
> {
>     const struct genl_info *info = genl_info_dump(cb);
>
> -   if (info->attrs[MPTCP_PM_ATTR_TOKEN])
> -       return mptcp_userspace_pm_dump_addr(msg, cb);
> -   return mptcp_pm_nl_dump_addr(msg, cb);
> +   if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
> +       return mptcp_pm_nl_dump_addr(msg, cb);
> +   return mptcp_userspace_pm_dump_addr(msg, cb);
> }
>
> int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
> {
> -   if (info->attrs[MPTCP_PM_ATTR_TOKEN])
> -       return mptcp_userspace_pm_set_flags(skb, info);
> -   return mptcp_pm_nl_set_flags(skb, info);
> +   if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
> +       return mptcp_pm_nl_set_flags(skb, info);
> +   return mptcp_userspace_pm_set_flags(skb, info);
> }
>
> void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 740a10d669f8..cc2b7c50d4d6 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -175,14 +175,15 @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
>
> static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
> {
> -   struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
>     struct mptcp_sock *msk;
> +   struct nlattr *token;
>
> -   if (!token) {
> +   if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN)) {
>         GENL_SET_ERR_MSG(info, "missing required token");

It is no longer needed to set the error message if the missing argument
has been already set.

Cheers,
Matt

>         return NULL;
>     }
>
> +   token = info->attrs[MPTCP_PM_ATTR_TOKEN];
>     msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
>     if (!msk) {
>         NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
> --
> 2.45.2

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.