From: Gang Yan <yangang@kylinos.cn>
The @max argument is never read in the function body. Remove it and the
MAX_TCP_KEEP* values passed by the TCP_KEEPIDLE/INTVL/KEEPCNT callers.
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
net/mptcp/sockopt.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index fcf6feb2a9eb..9c4faa0d82ef 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -632,7 +632,7 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
return ret;
}
-static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk, int max,
+static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk,
int (*set_val)(struct sock *, int),
int *msk_val, int val)
{
@@ -876,18 +876,15 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
ret = __mptcp_setsockopt_sol_tcp_nodelay(msk, val);
break;
case TCP_KEEPIDLE:
- ret = __mptcp_setsockopt_set_val(msk, MAX_TCP_KEEPIDLE,
- &tcp_sock_set_keepidle_locked,
+ ret = __mptcp_setsockopt_set_val(msk, &tcp_sock_set_keepidle_locked,
&msk->keepalive_idle, val);
break;
case TCP_KEEPINTVL:
- ret = __mptcp_setsockopt_set_val(msk, MAX_TCP_KEEPINTVL,
- &tcp_sock_set_keepintvl,
+ ret = __mptcp_setsockopt_set_val(msk, &tcp_sock_set_keepintvl,
&msk->keepalive_intvl, val);
break;
case TCP_KEEPCNT:
- ret = __mptcp_setsockopt_set_val(msk, MAX_TCP_KEEPCNT,
- &tcp_sock_set_keepcnt,
+ ret = __mptcp_setsockopt_set_val(msk, &tcp_sock_set_keepcnt,
&msk->keepalive_cnt,
val);
break;
--
2.43.0On Mon, 2026-07-27 at 10:28 +0800, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
>
> The @max argument is never read in the function body. Remove it and
> the
> MAX_TCP_KEEP* values passed by the TCP_KEEPIDLE/INTVL/KEEPCNT
> callers.
No, we shouldn't drop this "max" argument. Instead, we need to add some
code to check the "val" argument against the "max" argument in
__mptcp_setsockopt_set_val(), something like:
if (val < 1 || val > max)
return -EINVAL;
>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
> net/mptcp/sockopt.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index fcf6feb2a9eb..9c4faa0d82ef 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -632,7 +632,7 @@ static int
> mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
> return ret;
> }
>
> -static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk, int
> max,
> +static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk,
> int (*set_val)(struct sock *,
> int),
> int *msk_val, int val)
> {
> @@ -876,18 +876,15 @@ static int mptcp_setsockopt_sol_tcp(struct
> mptcp_sock *msk, int optname,
> ret = __mptcp_setsockopt_sol_tcp_nodelay(msk, val);
> break;
> case TCP_KEEPIDLE:
> - ret = __mptcp_setsockopt_set_val(msk,
> MAX_TCP_KEEPIDLE,
> -
> &tcp_sock_set_keepidle_locked,
> + ret = __mptcp_setsockopt_set_val(msk,
> &tcp_sock_set_keepidle_locked,
> &msk-
> >keepalive_idle, val);
> break;
> case TCP_KEEPINTVL:
> - ret = __mptcp_setsockopt_set_val(msk,
> MAX_TCP_KEEPINTVL,
> -
> &tcp_sock_set_keepintvl,
> + ret = __mptcp_setsockopt_set_val(msk,
> &tcp_sock_set_keepintvl,
> &msk-
> >keepalive_intvl, val);
> break;
> case TCP_KEEPCNT:
> - ret = __mptcp_setsockopt_set_val(msk,
> MAX_TCP_KEEPCNT,
> -
> &tcp_sock_set_keepcnt,
> + ret = __mptcp_setsockopt_set_val(msk,
> &tcp_sock_set_keepcnt,
> &msk-
> >keepalive_cnt,
> val);
> break;
July 27, 2026 at 2:37 PM, "Geliang Tang" <geliang@kernel.org mailto:geliang@kernel.org?to=%22Geliang%20Tang%22%20%3Cgeliang%40kernel.org%3E > wrote:
>
> On Mon, 2026-07-27 at 10:28 +0800, Gang Yan wrote:
>
> >
> > From: Gang Yan <yangang@kylinos.cn>
> >
> > The @max argument is never read in the function body. Remove it and
> > the
> > MAX_TCP_KEEP* values passed by the TCP_KEEPIDLE/INTVL/KEEPCNT
> > callers.
> >
> No, we shouldn't drop this "max" argument. Instead, we need to add some
> code to check the "val" argument against the "max" argument in
> __mptcp_setsockopt_set_val(), something like:
>
> if (val < 1 || val > max)
> return -EINVAL;
>
Hi Geliang
Thank you for the review and the suggestion to add a range check inside __mptcp_setsockopt_set_val().
But, I noticed that the underlying TCP helper functions already perform their
own parameter validation internally.
Given that these helpers already enforce the same bounds, I would like to ask
for your opinion: Is it still necessary to add an extra check in the MPTCP layer?
Thanks
Gang
> >
> > Signed-off-by: Gang Yan <yangang@kylinos.cn>
> > ---
> > net/mptcp/sockopt.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index fcf6feb2a9eb..9c4faa0d82ef 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -632,7 +632,7 @@ static int
> > mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
> > return ret;
> > }
> >
> > -static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk, int
> > max,
> > +static int __mptcp_setsockopt_set_val(struct mptcp_sock *msk,
> > int (*set_val)(struct sock *,
> > int),
> > int *msk_val, int val)
> > {
> > @@ -876,18 +876,15 @@ static int mptcp_setsockopt_sol_tcp(struct
> > mptcp_sock *msk, int optname,
> > ret = __mptcp_setsockopt_sol_tcp_nodelay(msk, val);
> > break;
> > case TCP_KEEPIDLE:
> > - ret = __mptcp_setsockopt_set_val(msk,
> > MAX_TCP_KEEPIDLE,
> > -
> > &tcp_sock_set_keepidle_locked,
> > + ret = __mptcp_setsockopt_set_val(msk,
> > &tcp_sock_set_keepidle_locked,
> > &msk-
> > keepalive_idle, val);
> > break;
> > case TCP_KEEPINTVL:
> > - ret = __mptcp_setsockopt_set_val(msk,
> > MAX_TCP_KEEPINTVL,
> > -
> > &tcp_sock_set_keepintvl,
> > + ret = __mptcp_setsockopt_set_val(msk,
> > &tcp_sock_set_keepintvl,
> > &msk-
> > keepalive_intvl, val);
> > break;
> > case TCP_KEEPCNT:
> > - ret = __mptcp_setsockopt_set_val(msk,
> > MAX_TCP_KEEPCNT,
> > -
> > &tcp_sock_set_keepcnt,
> > + ret = __mptcp_setsockopt_set_val(msk,
> > &tcp_sock_set_keepcnt,
> > &msk-
> > keepalive_cnt,
> > val);
> > break;
> >
>
© 2016 - 2026 Red Hat, Inc.