* [PATCH 1/1] net: ipv4: fix return value check in esp_remove_trailer()
@ 2023-07-17 14:49 Yuanjun Gong
2023-07-18 14:44 ` David Ahern
0 siblings, 1 reply; 5+ messages in thread
From: Yuanjun Gong @ 2023-07-17 14:49 UTC (permalink / raw)
To: Yuanjun Gong, Steffen Klassert, Herbert Xu, David Ahern, netdev
return an error number if an unexpected result is returned by
pskb_tirm() in esp_remove_trailer().
Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
---
net/ipv4/esp4.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index ba06ed42e428..0660bf2bdbae 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -732,7 +732,10 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
skb->csum = csum_block_sub(skb->csum, csumdiff,
skb->len - trimlen);
}
- pskb_trim(skb, skb->len - trimlen);
+ if (pskb_trim(skb, skb->len - trimlen)) {
+ ret = -EINVAL;
+ goto out;
+ }
ret = nexthdr[1];
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] net: ipv4: fix return value check in esp_remove_trailer()
2023-07-17 14:49 [PATCH 1/1] net: ipv4: fix return value check in esp_remove_trailer() Yuanjun Gong
@ 2023-07-18 14:44 ` David Ahern
2023-07-25 6:40 ` [PATCH v2 " Yuanjun Gong
0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2023-07-18 14:44 UTC (permalink / raw)
To: Yuanjun Gong, Steffen Klassert, Herbert Xu, netdev
On 7/17/23 8:49 AM, Yuanjun Gong wrote:
> return an error number if an unexpected result is returned by
> pskb_tirm() in esp_remove_trailer().
>
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
> net/ipv4/esp4.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index ba06ed42e428..0660bf2bdbae 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -732,7 +732,10 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
> skb->csum = csum_block_sub(skb->csum, csumdiff,
> skb->len - trimlen);
> }
> - pskb_trim(skb, skb->len - trimlen);
> + if (pskb_trim(skb, skb->len - trimlen)) {
> + ret = -EINVAL;
pskb_trim returns the error from ___pskb_trim; use it instead of
changing it here.
> + goto out;
> + }
>
> ret = nexthdr[1];
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/1] net: ipv4: fix return value check in esp_remove_trailer()
2023-07-18 14:44 ` David Ahern
@ 2023-07-25 6:40 ` Yuanjun Gong
2023-07-25 17:44 ` Alexander H Duyck
0 siblings, 1 reply; 5+ messages in thread
From: Yuanjun Gong @ 2023-07-25 6:40 UTC (permalink / raw)
To: dsahern; +Cc: herbert, netdev, ruc_gongyuanjun, steffen.klassert
return an error number if an unexpected result is returned by
pskb_tirm() in esp_remove_trailer().
Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
---
net/ipv4/esp4.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index ba06ed42e428..b435e3fe4dc6 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -732,7 +732,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
skb->csum = csum_block_sub(skb->csum, csumdiff,
skb->len - trimlen);
}
- pskb_trim(skb, skb->len - trimlen);
+ ret = pskb_trim(skb, skb->len - trimlen);
+ if (ret)
+ goto out;
ret = nexthdr[1];
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] net: ipv4: fix return value check in esp_remove_trailer()
2023-07-25 6:40 ` [PATCH v2 " Yuanjun Gong
@ 2023-07-25 17:44 ` Alexander H Duyck
2023-07-28 10:00 ` Herbert Xu
0 siblings, 1 reply; 5+ messages in thread
From: Alexander H Duyck @ 2023-07-25 17:44 UTC (permalink / raw)
To: Yuanjun Gong, dsahern; +Cc: herbert, netdev, steffen.klassert
On Tue, 2023-07-25 at 14:40 +0800, Yuanjun Gong wrote:
> return an error number if an unexpected result is returned by
> pskb_tirm() in esp_remove_trailer().
>
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
> net/ipv4/esp4.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index ba06ed42e428..b435e3fe4dc6 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -732,7 +732,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
> skb->csum = csum_block_sub(skb->csum, csumdiff,
> skb->len - trimlen);
> }
> - pskb_trim(skb, skb->len - trimlen);
> + ret = pskb_trim(skb, skb->len - trimlen);
> + if (ret)
> + goto out;
>
> ret = nexthdr[1];
>
In what case would you encounter this error? From what I can tell it
looks like there are checks in the callers, specifically the call to
pskb_may_pull() at the start of esp_input() that will go through and
automatically eliminate all the potential reasons for this to fail. So
I am not sure what the point is in adding exception handling for an
exception that is already handled.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] net: ipv4: fix return value check in esp_remove_trailer()
2023-07-25 17:44 ` Alexander H Duyck
@ 2023-07-28 10:00 ` Herbert Xu
0 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2023-07-28 10:00 UTC (permalink / raw)
To: Alexander H Duyck; +Cc: Yuanjun Gong, dsahern, netdev, steffen.klassert
On Tue, Jul 25, 2023 at 10:44:50AM -0700, Alexander H Duyck wrote:
> On Tue, 2023-07-25 at 14:40 +0800, Yuanjun Gong wrote:
> > return an error number if an unexpected result is returned by
> > pskb_tirm() in esp_remove_trailer().
> >
> > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> > ---
> > net/ipv4/esp4.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> > index ba06ed42e428..b435e3fe4dc6 100644
> > --- a/net/ipv4/esp4.c
> > +++ b/net/ipv4/esp4.c
> > @@ -732,7 +732,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
> > skb->csum = csum_block_sub(skb->csum, csumdiff,
> > skb->len - trimlen);
> > }
> > - pskb_trim(skb, skb->len - trimlen);
> > + ret = pskb_trim(skb, skb->len - trimlen);
> > + if (ret)
> > + goto out;
> >
> > ret = nexthdr[1];
> >
>
> In what case would you encounter this error? From what I can tell it
> looks like there are checks in the callers, specifically the call to
> pskb_may_pull() at the start of esp_input() that will go through and
> automatically eliminate all the potential reasons for this to fail. So
> I am not sure what the point is in adding exception handling for an
> exception that is already handled.
Good point. pskb_trim should never fail at this point because
we've already made the packet completely writeable.
Perhaps we could add a comment?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-28 10:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-17 14:49 [PATCH 1/1] net: ipv4: fix return value check in esp_remove_trailer() Yuanjun Gong
2023-07-18 14:44 ` David Ahern
2023-07-25 6:40 ` [PATCH v2 " Yuanjun Gong
2023-07-25 17:44 ` Alexander H Duyck
2023-07-28 10:00 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).