Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1235613 > unrolled thread
| Started by | Joe Stringer <joestringer@nicira.com> |
|---|---|
| First post | 2015-09-30 00:50 +0200 |
| Last post | 2015-09-30 16:50 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() Joe Stringer <joestringer@nicira.com> - 2015-09-30 00:50 +0200
Re: [PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() "Rustad, Mark D" <mark.d.rustad@intel.com> - 2015-09-30 00:50 +0200
Re: [PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() Joe Stringer <joestringer@nicira.com> - 2015-09-30 01:20 +0200
Re: [PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-30 16:50 +0200
| From | Joe Stringer <joestringer@nicira.com> |
|---|---|
| Date | 2015-09-30 00:50 +0200 |
| Subject | [PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() |
| Message-ID | <qebSG-3fs-15@gated-at.bofh.it> |
If ovs_fragment() was unable to fragment the skb due to an L2 header
that exceeds the supported length, skbs would be leaked. Fix the bug.
Fixes: 7f8a436 "openvswitch: Add conntrack action"
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
net/openvswitch/actions.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index e23a61c..e1afbd1 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -684,7 +684,7 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
{
if (skb_network_offset(skb) > MAX_L2_LEN) {
OVS_NLERR(1, "L2 header too long to fragment");
- return;
+ goto out;
}
if (ethertype == htons(ETH_P_IP)) {
@@ -708,8 +708,7 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
struct rt6_info ovs_rt;
if (!v6ops) {
- kfree_skb(skb);
- return;
+ goto out;
}
prepare_frag(vport, skb);
@@ -728,8 +727,14 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.",
ovs_vport_name(vport), ntohs(ethertype), mru,
vport->dev->mtu);
- kfree_skb(skb);
+ goto out;
}
+
+ skb = NULL;
+
+out:
+ if (skb)
+ kfree_skb(skb);
}
static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | "Rustad, Mark D" <mark.d.rustad@intel.com> |
|---|---|
| Date | 2015-09-30 00:50 +0200 |
| Message-ID | <qebSH-3fs-25@gated-at.bofh.it> |
| In reply to | #1235613 |
[Multipart message — attachments visible in raw view] — view raw
> On Sep 29, 2015, at 3:39 PM, Joe Stringer <joestringer@nicira.com> wrote: > > @@ -728,8 +727,14 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru, > WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.", > ovs_vport_name(vport), ntohs(ethertype), mru, > vport->dev->mtu); > - kfree_skb(skb); > + goto out; > } > + > + skb = NULL; > + > +out: > + if (skb) > + kfree_skb(skb); > } > > static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port, Wouldn't that hunk be better as: @@ -728,8 +727,13 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru, WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.", ovs_vport_name(vport), ntohs(ethertype), mru, vport->dev->mtu); - kfree_skb(skb); + goto out; } + + return; + +out: + kfree_skb(skb); } static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port, -- Mark Rustad, Networking Division, Intel Corporation
[toc] | [prev] | [next] | [standalone]
| From | Joe Stringer <joestringer@nicira.com> |
|---|---|
| Date | 2015-09-30 01:20 +0200 |
| Message-ID | <qeclI-438-11@gated-at.bofh.it> |
| In reply to | #1235616 |
On 29 September 2015 at 15:48, Rustad, Mark D <mark.d.rustad@intel.com> wrote: >> On Sep 29, 2015, at 3:39 PM, Joe Stringer <joestringer@nicira.com> wrote: >> >> @@ -728,8 +727,14 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru, >> WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.", >> ovs_vport_name(vport), ntohs(ethertype), mru, >> vport->dev->mtu); >> - kfree_skb(skb); >> + goto out; >> } >> + >> + skb = NULL; >> + >> +out: >> + if (skb) >> + kfree_skb(skb); >> } >> >> static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port, > > Wouldn't that hunk be better as: > > @@ -728,8 +727,13 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru, > WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.", > ovs_vport_name(vport), ntohs(ethertype), mru, > vport->dev->mtu); > - kfree_skb(skb); > + goto out; > } > + > + return; > + > +out: > + kfree_skb(skb); > } > > static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port, > > -- > Mark Rustad, Networking Division, Intel Corporation Sure thing, I'll roll this change in to a v2 when the rest of the series is reviewed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-09-30 16:50 +0200 |
| Message-ID | <qeqRH-7Z1-3@gated-at.bofh.it> |
| In reply to | #1235613 |
Hello.
On 09/30/2015 01:39 AM, Joe Stringer wrote:
> If ovs_fragment() was unable to fragment the skb due to an L2 header
> that exceeds the supported length, skbs would be leaked. Fix the bug.
>
> Fixes: 7f8a436 "openvswitch: Add conntrack action"
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> net/openvswitch/actions.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index e23a61c..e1afbd1 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
[...]
> @@ -728,8 +727,14 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
> WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.",
> ovs_vport_name(vport), ntohs(ethertype), mru,
> vport->dev->mtu);
> - kfree_skb(skb);
> + goto out;
> }
> +
> + skb = NULL;
I'd just return here.
> +
> +out:
> + if (skb)
> + kfree_skb(skb);
kfree_skb() checks for NULL.
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web