| Message ID | 637fc17e2c0f833e820f7f8097fdc1212b4c968f.1622719886.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | a88964048fecc1c1e19e371ce4b9029510ecf6f9 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [v2,mptcp-net] mptcp: do not warn on bad input from the network | expand |
On Thu, 3 Jun 2021, Paolo Abeni wrote: > warn_bad_map() produces a kernel WARN on bad input coming > from the network. Use pr_debug() to avoid spamming the system > log. > > Additionally, 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") > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/107 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > I think we should really abvoid the warn here. Later patches will add > MIB counters for the relevant event > --- > net/mptcp/subflow.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > Thanks for the patch Paolo. Looks good. Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > index 5ef31aa16cc1..259f99b61c35 100644 > --- a/net/mptcp/subflow.c > +++ b/net/mptcp/subflow.c > @@ -787,10 +787,10 @@ static u64 expand_seq(u64 old_seq, u16 old_data_len, u64 seq) > return seq | ((old_seq + old_data_len + 1) & GENMASK_ULL(63, 32)); > } > > -static void warn_bad_map(struct mptcp_subflow_context *subflow, u32 ssn) > +static void dbg_bad_map(struct mptcp_subflow_context *subflow, u32 ssn) > { > - WARN_ONCE(1, "Bad mapping: ssn=%d map_seq=%d map_data_len=%d", > - ssn, subflow->map_subflow_seq, subflow->map_data_len); > + pr_debug("Bad mapping: ssn=%d map_seq=%d map_data_len=%d", > + ssn, subflow->map_subflow_seq, subflow->map_data_len); > } > > static bool skb_is_fully_mapped(struct sock *ssk, struct sk_buff *skb) > @@ -815,13 +815,13 @@ static bool validate_mapping(struct sock *ssk, struct sk_buff *skb) > /* Mapping covers data later in the subflow stream, > * currently unsupported. > */ > - warn_bad_map(subflow, ssn); > + dbg_bad_map(subflow, ssn); > return false; > } > 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); > + dbg_bad_map(subflow, ssn); > return false; > } > return true; > -- > 2.26.3 > > > -- Mat Martineau Intel
Hi Paolo, Mat, On 03/06/2021 13:31, Paolo Abeni wrote: > warn_bad_map() produces a kernel WARN on bad input coming > from the network. Use pr_debug() to avoid spamming the system > log. > > Additionally, 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") > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/107 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> Thank you for the patch and the review! - a88964048fec: mptcp: do not warn on bad input from the network - Results: 02c88cfc4215..102ad1154235 Builds and tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210604T200651 https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210604T200651 Cheers, Matt
Hi Paolo, Mat, On 03/06/2021 13:31, Paolo Abeni wrote: > warn_bad_map() produces a kernel WARN on bad input coming > from the network. Use pr_debug() to avoid spamming the system > log. > > Additionally, 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") > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/107 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> Thank you for the patch and the review! - a88964048fec: mptcp: do not warn on bad input from the network - Results: 02c88cfc4215..102ad1154235 Builds and tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210604T200651 https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210604T200651 Cheers, Matt
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 5ef31aa16cc1..259f99b61c35 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -787,10 +787,10 @@ static u64 expand_seq(u64 old_seq, u16 old_data_len, u64 seq) return seq | ((old_seq + old_data_len + 1) & GENMASK_ULL(63, 32)); } -static void warn_bad_map(struct mptcp_subflow_context *subflow, u32 ssn) +static void dbg_bad_map(struct mptcp_subflow_context *subflow, u32 ssn) { - WARN_ONCE(1, "Bad mapping: ssn=%d map_seq=%d map_data_len=%d", - ssn, subflow->map_subflow_seq, subflow->map_data_len); + pr_debug("Bad mapping: ssn=%d map_seq=%d map_data_len=%d", + ssn, subflow->map_subflow_seq, subflow->map_data_len); } static bool skb_is_fully_mapped(struct sock *ssk, struct sk_buff *skb) @@ -815,13 +815,13 @@ static bool validate_mapping(struct sock *ssk, struct sk_buff *skb) /* Mapping covers data later in the subflow stream, * currently unsupported. */ - warn_bad_map(subflow, ssn); + dbg_bad_map(subflow, ssn); return false; } 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); + dbg_bad_map(subflow, ssn); return false; } return true;
warn_bad_map() produces a kernel WARN on bad input coming from the network. Use pr_debug() to avoid spamming the system log. Additionally, 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") Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/107 Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- I think we should really abvoid the warn here. Later patches will add MIB counters for the relevant event --- net/mptcp/subflow.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)