| Message ID | 20231006114106.3982925-1-make_ruc2021@163.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: xfrm: fix return value check in ipcomp_compress | expand |
Ma Ke <make_ruc2021@163.com> wrote: > In ipcomp_compress, to avoid an unexpected result returned by > pskb_trim, we should check the return value of pskb_trim(). > > Signed-off-by: Ma Ke <make_ruc2021@163.com> > --- > net/xfrm/xfrm_ipcomp.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c > index 9c0fa0e1786a..5f2e6edadf48 100644 > --- a/net/xfrm/xfrm_ipcomp.c > +++ b/net/xfrm/xfrm_ipcomp.c > @@ -144,7 +144,9 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb) > memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen); > local_bh_enable(); > > - pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); > + err = pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); > + if (unlikely(err)) > + goto out; This can't be right, this now calls local_bh_enable() twice.
Florian Westphal <fw@strlen.de> wrote: > Ma Ke <make_ruc2021@163.com> wrote: > > In ipcomp_compress, to avoid an unexpected result returned by > > pskb_trim, we should check the return value of pskb_trim(). > > > > Signed-off-by: Ma Ke <make_ruc2021@163.com> > > --- > > net/xfrm/xfrm_ipcomp.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c > > index 9c0fa0e1786a..5f2e6edadf48 100644 > > --- a/net/xfrm/xfrm_ipcomp.c > > +++ b/net/xfrm/xfrm_ipcomp.c > > @@ -144,7 +144,9 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb) > > memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen); > > local_bh_enable(); > > > > - pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); > > + err = pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); > > + if (unlikely(err)) > > + goto out; > > This can't be right, this now calls local_bh_enable() twice. Furthermore, looking at this: 1. skb went through skb_linearize_cow() before, so no paged data anymore 2. Right before there is a check to bail in case compression inflated packet size. IOW, this pskb_trim cannot fail, it boils down to __skb_trim().
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 9c0fa0e1786a..5f2e6edadf48 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -144,7 +144,9 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb) memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen); local_bh_enable(); - pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); + err = pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr)); + if (unlikely(err)) + goto out; return 0; out:
In ipcomp_compress, to avoid an unexpected result returned by pskb_trim, we should check the return value of pskb_trim(). Signed-off-by: Ma Ke <make_ruc2021@163.com> --- net/xfrm/xfrm_ipcomp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)