[PATCH mptcp-net] mptcp: let warn_bad_map report relevant values

[PATCH mptcp-net] mptcp: let warn_bad_map report relevant values

[Paolo Abeni]

When the right bound check fails, warn_bad_map() reports
the wrong ssn value, let's fix it.

Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/subflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6b1cd4257edf..726bc3d083fa 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -821,7 +821,7 @@ static bool validate_mapping(struct sock *ssk, struct sk_buff *skb)
 	if (unlikely(!before(ssn, subflow->map_subflow_seq +
 				  subflow->map_data_len))) {
 		/* Mapping does covers past subflow data, invalid */
-		warn_bad_map(subflow, ssn + skb->len);
+		warn_bad_map(subflow, ssn);
 		return false;
 	}
 	return true;
-- 
2.26.3

Re: [PATCH mptcp-net] mptcp: let warn_bad_map report relevant values

[Mat Martineau]

On Wed, 26 May 2021, Paolo Abeni wrote:

> When the right bound check fails, warn_bad_map() reports
> the wrong ssn value, let's fix it.
>
> Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/subflow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 6b1cd4257edf..726bc3d083fa 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -821,7 +821,7 @@ static bool validate_mapping(struct sock *ssk, struct sk_buff *skb)
> 	if (unlikely(!before(ssn, subflow->map_subflow_seq +
> 				  subflow->map_data_len))) {
> 		/* Mapping does covers past subflow data, invalid */
> -		warn_bad_map(subflow, ssn + skb->len);
> +		warn_bad_map(subflow, ssn);
> 		return false;
> 	}
> 	return true;
> -- 
> 2.26.3

I agree, ssn is more helpful information here.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: [PATCH mptcp-net] mptcp: let warn_bad_map report relevant values

[Paolo Abeni]

On Wed, 2021-05-26 at 15:55 -0700, Mat Martineau wrote:
> On Wed, 26 May 2021, Paolo Abeni wrote:
> 
> > When the right bound check fails, warn_bad_map() reports
> > the wrong ssn value, let's fix it.
> > 
> > Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > net/mptcp/subflow.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index 6b1cd4257edf..726bc3d083fa 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -821,7 +821,7 @@ static bool validate_mapping(struct sock *ssk, struct sk_buff *skb)
> > 	if (unlikely(!before(ssn, subflow->map_subflow_seq +
> > 				  subflow->map_data_len))) {
> > 		/* Mapping does covers past subflow data, invalid */
> > -		warn_bad_map(subflow, ssn + skb->len);
> > +		warn_bad_map(subflow, ssn);
> > 		return false;
> > 	}
> > 	return true;
> > -- 
> > 2.26.3
> 
> I agree, ssn is more helpful information here.
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

I see this one has not been applied yet. Evil/bugged peer (or even
fallback with no infinite mapping) can trigger this WARN. It looks like
this is the only WARN() leftover from issues/107.

I'll send a v2 addionally replacing the WARN with a pr_debug().

/P