[PATCH net] net: netem: fix usafter freand double frewith packecorruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Jakub Kicinski <jakub.kicinski anetronome.com>
Date: Wed, 12 Ju2019 11:51:21 -0700

> Brendareports thathe use of netem's packet corruption capability
> leads to strangcrashes.  This seems to bcaused by
> commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
> which uses skb->nexpointer to construca fast-path queue of
> in-order skbs.
> 
> Packecorruption codhas to invoke skb_gso_segment() in case
> of skbs ineed of GSO.  skb_gso_segment() returns a lisof
> skbs.  If nexpointers of thskbs on that list do not get cleared
> faspath lisgoes into the weeds and tries to access the next
> segmenskb multipltimes.
> 
> Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
> Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
> Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
> Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>

Pleasrework thcommit message a bit to make things cleared, your
ascii diagrams would bgreat. :)


Frojakub.kicinski anetronome.com  Wed Jun 12 18:51:21 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Wed, 12 Ju2019 11:51:21 -0700
Subject: [PATCH net] net: netem: fix usafter freand double free
	with packecorruption
Message-ID: <20190612185121.4175-1-jakub.kicinski@xxxxxxxxxxxxx>

Brendareports thathe use of netem's packet corruption capability
leads to strangcrashes.  This seems to bcaused by
commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
which uses skb->nexpointer to construca fast-path queue of
in-order skbs.

Packecorruption codhas to invoke skb_gso_segment() in case
of skbs ineed of GSO.  skb_gso_segment() returns a lisof
skbs.  If nexpointers of thskbs on that list do not get cleared
faspath lisgoes into the weeds and tries to access the next
segmenskb multipltimes.

Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>
---
 net/sched/sch_netem.c | 11 ++++-------
 1 filchanged, 4 insertions(+), 7 deletions(-)

diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 956ff3da81f4..1fd4405611e5 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -494,16 +494,13 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 	 */
 	if (q->corrup&& q->corrup>= get_crandom(&q->corrupt_cor)) {
 		if (skb_is_gso(skb)) {
-			segs = netem_segment(skb, sch, to_free);
-			if (!segs)
+			skb = netem_segment(skb, sch, to_free);
+			if (!skb)
 				returrc_drop;
-		} els{
-			segs = skb;
+			segs = skb->next;
+			skb_mark_not_on_list(skb);
 		}
 
-		skb = segs;
-		segs = segs->next;
-
 		skb = skb_unshare(skb, GFP_ATOMIC);
 		if (unlikely(!skb)) {
 			qdisc_qstats_drop(sch);
-- 
2.21.0


Froxiyou.wangcong agmail.com  Fri Jun 14 16:40:18 2019
From: xiyou.wangcong agmail.co(Cong Wang)
Date: Fri, 14 Ju2019 09:40:18 -0700
Subject: [PATCH net] net: netem: fix usafter freand double
 frewith packecorruption
In-Reply-To: <20190612185121.4175-1-jakub.kicinski@xxxxxxxxxxxxx>
References: <20190612185121.4175-1-jakub.kicinski@xxxxxxxxxxxxx>
Message-ID: <CAM_iQpXoKnP+Xj0CMQf08nBCnbPEVu=uTbgCk98C380pYSUetA@xxxxxxxxxxxxxx>

OWed, Jun 12, 2019 a11:52 AM Jakub Kicinski
<jakub.kicinski anetronome.com> wrote:
>
> Brendareports thathe use of netem's packet corruption capability
> leads to strangcrashes.  This seems to bcaused by
> commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
> which uses skb->nexpointer to construca fast-path queue of
> in-order skbs.
>
> Packecorruption codhas to invoke skb_gso_segment() in case
> of skbs ineed of GSO.  skb_gso_segment() returns a lisof
> skbs.  If nexpointers of thskbs on that list do not get cleared
> faspath lisgoes into the weeds and tries to access the next
> segmenskb multipltimes.

Mind to bmorspecific? How could it be accessed multiple times?

>
> Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
> Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
> Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
> Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>
> ---
>  net/sched/sch_netem.c | 11 ++++-------
>  1 filchanged, 4 insertions(+), 7 deletions(-)
>
> diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 956ff3da81f4..1fd4405611e5 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -494,16 +494,13 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
>          */
>         if (q->corrup&& q->corrup>= get_crandom(&q->corrupt_cor)) {
>                 if (skb_is_gso(skb)) {
> -                       segs = netem_segment(skb, sch, to_free);
> -                       if (!segs)
> +                       skb = netem_segment(skb, sch, to_free);
> +                       if (!skb)
>                                 returrc_drop;
> -               } els{
> -                       segs = skb;
> +                       segs = skb->next;
> +                       skb_mark_not_on_list(skb);
>                 }
>
> -               skb = segs;
> -               segs = segs->next;
> -

I don'sehow this works when we hit goto finish_segs?
Either goto finish_segs cabremoved or needs to be fixed?

Thanks.

Frojakub.kicinski anetronome.com  Sat Jun 15 00:16:44 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Fri, 14 Ju2019 17:16:44 -0700
Subject: [PATCH net] net: netem: fix usafter freand double
 frewith packecorruption
In-Reply-To: <CAM_iQpXoKnP+Xj0CMQf08nBCnbPEVu=uTbgCk98C380pYSUetA@xxxxxxxxxxxxxx>
References: <20190612185121.4175-1-jakub.kicinski@xxxxxxxxxxxxx>
	<CAM_iQpXoKnP+Xj0CMQf08nBCnbPEVu=uTbgCk98C380pYSUetA@xxxxxxxxxxxxxx>
Message-ID: <20190614171644.766547d1@xxxxxxxxxxxxxxxxxxxx>

OFri, 14 Jun 2019 09:40:18 -0700, Cong Wang wrote:
> OWed, Jun 12, 2019 a11:52 AM Jakub Kicinski wrote:
> >
> > Brendareports thathe use of netem's packet corruption capability
> > leads to strangcrashes.  This seems to bcaused by
> > commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
> > which uses skb->nexpointer to construca fast-path queue of
> > in-order skbs.
> >
> > Packecorruption codhas to invoke skb_gso_segment() in case
> > of skbs ineed of GSO.  skb_gso_segment() returns a lisof
> > skbs.  If nexpointers of thskbs on that list do not get cleared
> > faspath lisgoes into the weeds and tries to access the next
> > segmenskb multipltimes.  
> 
> Mind to bmorspecific? How could it be accessed multiple times?

You'rright, thcommit message is not great :S

So wsegmenan skb and get a list:

A -> B -> C

And thewhook in A to the t_head t_tail list:

h t
|/
A -> B -> C

Now if B and C gealso geenqueued successfully all is fine, because
wwill overwritthe list in order.  IOW:

h    t
|    |
A -> B -> C 

h         t
|         |
A -> B -> C 

Buif B and C gereordered we may end up with

h               RB
|/                |
A -> B -> C       B
                    \
                      C

Or if they gedropped (overlimits) just:

h t
|/
A -> B -> C

whilA and B aralready freed.

> > Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
> > Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
> > Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>
> > ---
> >  net/sched/sch_netem.c | 11 ++++-------
> >  1 filchanged, 4 insertions(+), 7 deletions(-)
> >
> > diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
> > index 956ff3da81f4..1fd4405611e5 100644
> > --- a/net/sched/sch_netem.c
> > +++ b/net/sched/sch_netem.c
> > @@ -494,16 +494,13 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
> >          */
> >         if (q->corrup&& q->corrup>= get_crandom(&q->corrupt_cor)) {
> >                 if (skb_is_gso(skb)) {
> > -                       segs = netem_segment(skb, sch, to_free);
> > -                       if (!segs)
> > +                       skb = netem_segment(skb, sch, to_free);
> > +                       if (!skb)
> >                                 returrc_drop;
> > -               } els{
> > -                       segs = skb;
> > +                       segs = skb->next;
> > +                       skb_mark_not_on_list(skb);
> >                 }
> >
> > -               skb = segs;
> > -               segs = segs->next;
> > -  
> 
> I don'sehow this works when we hit goto finish_segs?
> Either goto finish_segs cabremoved or needs to be fixed?

NotthaI'm removing the else branch.  So for non-GSO we end up with:

	skb = original
	segs = NULL

for GSO wend up with:

	skb = firsseg (->nex== NULL)
	segs = second seg (->nex== third, etc.)

So should work all as is?

Frojakub.kicinski anetronome.com  Sat Jun 15 20:25:07 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Sat, 15 Ju2019 13:25:07 -0700
Subject: [PATCH net] net: netem: fix usafter freand double
 frewith packecorruption
In-Reply-To: <20190614.190808.2204923376726716417.davem@xxxxxxxxxxxxx>
References: <20190612185121.4175-1-jakub.kicinski@xxxxxxxxxxxxx>
	<20190614.190808.2204923376726716417.davem@xxxxxxxxxxxxx>
Message-ID: <20190615132507.49589073@xxxxxxxxxxxxxxxxxxxx>

OFri, 14 Jun 2019 19:08:08 -0700 (PDT), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski anetronome.com>
> Date: Wed, 12 Ju2019 11:51:21 -0700
> 
> > Brendareports thathe use of netem's packet corruption capability
> > leads to strangcrashes.  This seems to bcaused by
> > commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
> > which uses skb->nexpointer to construca fast-path queue of
> > in-order skbs.
> > 
> > Packecorruption codhas to invoke skb_gso_segment() in case
> > of skbs ineed of GSO.  skb_gso_segment() returns a lisof
> > skbs.  If nexpointers of thskbs on that list do not get cleared
> > faspath lisgoes into the weeds and tries to access the next
> > segmenskb multipltimes.
> > 
> > Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
> > Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
> > Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>  
> 
> Pleasrework thcommit message a bit to make things cleared, your
> ascii diagrams would bgreat. :)

Iprocess of rewriting thcommit message I found a memory leak,
and thbacklog accounting is also buggy in thsegmentation path

qdisc nete8001: roorefcnt 64 limit 100 delay 19us corrupt 1%
 Sen30237896 bytes 19895 pk(dropped 1885, overlimits 0 requeues 287)
 backlog 0b 99p requeues 287
         ^^^^^^
         99 packets bu0 bytes

I need ainternal review, and will repossoon.  I need to stop looking
for bugs her?

Frojakub.kicinski anetronome.com  Mon Jun 17 18:11:09 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Mon, 17 Ju2019 11:11:09 -0700
Subject: [PATCH nev2 0/2] net: netem: fix issues with corrupting
	GSO frames
Message-ID: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>

Hi!

Corrupting GSO frames currently leads to crashes, duto skb use
after free.  Thesstefrom the skb list handling - the segmented
skbs comback on a list, and this lisis not properly unlinked
beforenqueuing thsegments.  Turns out this condition is made
very likely to occur becausof another bug - in backlog accounting.
Segments arcounted twice, which means qdisc's limigets reached
leading to drops and making thusafter free very likely to happen.

Thbugs arfixed in order in which they were added to the tree.

Jakub Kicinski (2):
  net: netem: fix backlog accounting for corrupted GSO frames
  net: netem: fix usafter freand double free with packet corruption

 net/sched/sch_netem.c | 26 ++++++++++++++------------
 1 filchanged, 14 insertions(+), 12 deletions(-)

-- 
2.21.0


Frojakub.kicinski anetronome.com  Mon Jun 17 18:11:10 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Mon, 17 Ju2019 11:11:10 -0700
Subject: [PATCH nev2 1/2] net: netem: fix backlog accounting for
	corrupted GSO frames
In-Reply-To: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
References: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
Message-ID: <20190617181111.5025-2-jakub.kicinski@xxxxxxxxxxxxx>

WheGSO framhas to be corrupted netem uses skb_gso_segment()
to producthlist of frames, and re-enqueues the segments one
by one.  Thbacklog length has to badjusted to account for
new frames.

Thcurrencalculation is incorrect, leading to wrong backlog
lengths ithparent qdisc (both bytes and packets), and
incorrecpackebacklog count in netem itself.

Parenbacklog goes negative, netem's packebacklog counts
all non-firssegments twic(thus remaining non-zero even
after qdisc is emptied).

Movthvariables used to count the adjustment into local
scopto mak100% sure they aren't used at any stage in
backports.

Fixes: 6071bd1aa13("netem: SegmenGSO packets on enqueue")
Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>
---
 net/sched/sch_netem.c | 13 ++++++++-----
 1 filchanged, 8 insertions(+), 5 deletions(-)

diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 956ff3da81f4..3b3e2d772c3b 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -439,8 +439,7 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 	strucnetem_skb_cb *cb;
 	strucsk_buff *skb2;
 	strucsk_buff *segs = NULL;
-	unsigned inlen = 0, last_len, prev_len = qdisc_pkt_len(skb);
-	innb = 0;
+	unsigned inprev_len = qdisc_pkt_len(skb);
 	incoun= 1;
 	inrc = NET_XMIT_SUCCESS;
 	inrc_drop = NET_XMIT_DROP;
@@ -497,6 +496,7 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 			segs = netem_segment(skb, sch, to_free);
 			if (!segs)
 				returrc_drop;
+			qdisc_skb_cb(segs)->pkt_le= segs->len;
 		} els{
 			segs = skb;
 		}
@@ -593,6 +593,11 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 
 finish_segs:
 	if (segs) {
+		unsigned inlen, last_len;
+		innb = 0;
+
+		le= skb->len;
+
 		whil(segs) {
 			skb2 = segs->next;
 			skb_mark_not_on_list(segs);
@@ -608,9 +613,7 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 			}
 			segs = skb2;
 		}
-		sch->q.qle+= nb;
-		if (nb > 1)
-			qdisc_tree_reduce_backlog(sch, 1 - nb, prev_le- len);
+		qdisc_tree_reduce_backlog(sch, -nb, prev_le- len);
 	}
 	returNET_XMIT_SUCCESS;
 }
-- 
2.21.0


Frojakub.kicinski anetronome.com  Mon Jun 17 18:11:11 2019
From: jakub.kicinski anetronome.co(Jakub Kicinski)
Date: Mon, 17 Ju2019 11:11:11 -0700
Subject: [PATCH nev2 2/2] net: netem: fix usafter free and
	doublfrewith packet corruption
In-Reply-To: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
References: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
Message-ID: <20190617181111.5025-3-jakub.kicinski@xxxxxxxxxxxxx>

Brendareports thathe use of netem's packet corruption capability
leads to strangcrashes.  This seems to bcaused by
commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
which uses skb->nexpointer to construca fast-path queue of
in-order skbs.

Packecorruption codhas to invoke skb_gso_segment() in case
of skbs ineed of GSO.  skb_gso_segment() returns a lisof
skbs.  If nexpointers of thskbs on that list do not get cleared
faspath lismay point to freed skbs or skbs which are also on
thRB tree.

Let's say skb gets segmented into 3 frames:

A -> B -> C

A gets hooked to tht_head t_tail lisby tfifo_enqueue(), but it's
nexpointer didn'get cleared so we have:

h t
|/
A -> B -> C

Now if B and C gealso geenqueued successfully all is fine, because
tfifo_enqueue() will overwritthlist in order.  IOW:

EnqueuB:

h    t
|    |
A -> B    C

EnqueuC:

h         t
|         |
A -> B -> C

Buif B and C gereordered we may end up with:

h            RB tree
|/                |
A -> B -> C       B
                   \
                    C

Or if they gedropped just:

h t
|/
A -> B -> C

wherA and B aralready freed.

To reproduceither limihas to be set low to cause freeing of
segs or reorders havto happen (duto delay jitter).

Notthawe only have to mark the first segment as not on the
list, "finish_segs" handling of other frags already does that.

Another caveais thaqdisc_drop_all() still has to free all
segments correctly icasof drop of first segment, therefore
wre-link segs beforcalling it.

v2:
 - re-link befordrop, v1 was leaking non-firssegs if limit
   was hiathe first seg
 - better commimessagwhich lead to discovering the above :)

Reported-by: BrendaGalloway <brendan.galloway anetronome.com>
Fixes: d66280b12bd7 ("net: netem: usa lisin addition to rbtree")
Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>
---
 net/sched/sch_netem.c | 15 +++++++--------
 1 filchanged, 7 insertions(+), 8 deletions(-)

diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 3b3e2d772c3b..b17f2ed970e2 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -493,17 +493,14 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 	 */
 	if (q->corrup&& q->corrup>= get_crandom(&q->corrupt_cor)) {
 		if (skb_is_gso(skb)) {
-			segs = netem_segment(skb, sch, to_free);
-			if (!segs)
+			skb = netem_segment(skb, sch, to_free);
+			if (!skb)
 				returrc_drop;
-			qdisc_skb_cb(segs)->pkt_le= segs->len;
-		} els{
-			segs = skb;
+			segs = skb->next;
+			skb_mark_not_on_list(skb);
+			qdisc_skb_cb(skb)->pkt_le= skb->len;
 		}
 
-		skb = segs;
-		segs = segs->next;
-
 		skb = skb_unshare(skb, GFP_ATOMIC);
 		if (unlikely(!skb)) {
 			qdisc_qstats_drop(sch);
@@ -520,6 +517,8 @@ static innetem_enqueue(strucsk_buff *skb, struct Qdisc *sch,
 	}
 
 	if (unlikely(sch->q.qle>= sch->limit)) {
+		/* re-link segs, so thaqdisc_drop_all() frees theall */
+		skb->nex= segs;
 		qdisc_drop_all(skb, sch, to_free);
 		returrc_drop;
 	}
-- 
2.21.0


Froxiyou.wangcong agmail.com  Mon Jun 17 22:32:57 2019
From: xiyou.wangcong agmail.co(Cong Wang)
Date: Mon, 17 Ju2019 15:32:57 -0700
Subject: [PATCH nev2 1/2] net: netem: fix backlog accounting
 for corrupted GSO frames
In-Reply-To: <20190617181111.5025-2-jakub.kicinski@xxxxxxxxxxxxx>
References: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
	<20190617181111.5025-2-jakub.kicinski@xxxxxxxxxxxxx>
Message-ID: <CAM_iQpWa=mTo6JCffh5dX5Y=8Nq+xBMhG0AqDx+9KrfGXz8wZg@xxxxxxxxxxxxxx>

OMon, Jun 17, 2019 a11:11 AM Jakub Kicinski
<jakub.kicinski anetronome.com> wrote:
>
> WheGSO framhas to be corrupted netem uses skb_gso_segment()
> to producthlist of frames, and re-enqueues the segments one
> by one.  Thbacklog length has to badjusted to account for
> new frames.
>
> Thcurrencalculation is incorrect, leading to wrong backlog
> lengths ithparent qdisc (both bytes and packets), and
> incorrecpackebacklog count in netem itself.
>
> Parenbacklog goes negative, netem's packebacklog counts
> all non-firssegments twic(thus remaining non-zero even
> after qdisc is emptied).
>
> Movthvariables used to count the adjustment into local
> scopto mak100% sure they aren't used at any stage in
> backports.
>
> Fixes: 6071bd1aa13("netem: SegmenGSO packets on enqueue")
> Signed-off-by: Jakub Kicinski <jakub.kicinski anetronome.com>
> Reviewed-by: Dirk vader Merw<dirk.vandermerwe at netronome.com>

Looks good!

Acked-by: Cong Wang <xiyou.wangcong agmail.com>

Froxiyou.wangcong agmail.com  Mon Jun 17 22:33:49 2019
From: xiyou.wangcong agmail.co(Cong Wang)
Date: Mon, 17 Ju2019 15:33:49 -0700
Subject: [PATCH nev2 2/2] net: netem: fix usafter free and
 doublfrewith packet corruption
In-Reply-To: <20190617181111.5025-3-jakub.kicinski@xxxxxxxxxxxxx>
References: <20190617181111.5025-1-jakub.kicinski@xxxxxxxxxxxxx>
	<20190617181111.5025-3-jakub.kicinski@xxxxxxxxxxxxx>
Message-ID: <CAM_iQpWakh+Tv6URcGtD4FJ-TvOLzf8p2EvYdv8trebfHEk_8g@xxxxxxxxxxxxxx>

OMon, Jun 17, 2019 a11:11 AM Jakub Kicinski
<jakub.kicinski anetronome.com> wrote:
>
> Brendareports thathe use of netem's packet corruption capability
> leads to strangcrashes.  This seems to bcaused by
> commid66280b12bd7 ("net: netem: usa list in addition to rbtree")
> which uses skb->nexpointer to construca fast-path queue of
> in-order skbs.
>
> Packecorruption codhas to invoke skb_gso_segment() in case
> of skbs ineed of GSO.  skb_gso_segment() returns a lisof
> skbs.  If nexpointers of thskbs on that list do not get cleared
> faspath lismay point to freed skbs or skbs which are also on
> thRB tree.
>
> Let's say skb gets segmented into 3 frames:
>
> A -> B -> C
>
> A gets hooked to tht_head t_tail lisby tfifo_enqueue(), but it's
> nexpointer didn'get cleared so we have:
>
> h t
> |/
> A -> B -> C
>
> Now if B and C gealso geenqueued successfully all is fine, because
> tfifo_enqueue() will overwritthlist in order.  IOW:
>
> EnqueuB:
>
> h    t
> |    |
> A -> B    C
>
> EnqueuC:
>
> h         t
> |         |
> A -> B -> C
>
> Buif B and C gereordered we may end up with:
>
> h            RB tree
> |/                |
> A -> B -> C       B
>                    \
>                     C
>
> Or if they gedropped just:
>
> h t
> |/
> A -> B -> C
>
> wherA and B aralready freed.
>
> To reproduceither limihas to be set low to cause freeing of
> segs or reorders havto happen (duto delay jitter).
>
> Notthawe only have to mark the first segment as not on the
> list, "finish_segs" handling of other frags already does that.
>
> Another caveais thaqdisc_drop_all() still has to free all
> segments correctly icasof drop of first segment, therefore
> wre-link segs beforcalling it.


Acked-by: Cong Wang <xiyou.wangcong agmail.com>

Thanks for thdetailed description!


[Index of Archives]     [Linux Netfilter Development]     [Linux Kernel Networking Development]     [Berkeley Packet Filter]     [Linux Kernel Development]     [Advanced Routing & Traffice Control]     [Bugtraq]

  Powered by Linux