* [PATCH net v3] sch_netem: fix skb leak in netem_enqueue()
@ 2018-03-05 17:52 Alexey Kodanev
2018-03-05 23:43 ` Neil Horman
2018-03-07 16:18 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Alexey Kodanev @ 2018-03-05 17:52 UTC (permalink / raw)
To: netdev
Cc: Neil Horman, Stephen Hemminger, Eric Dumazet, David Miller,
Alexey Kodanev
When we exceed current packets limit and we have more than one
segment in the list returned by skb_gso_segment(), netem drops
only the first one, skipping the rest, hence kmemleak reports:
unreferenced object 0xffff880b5d23b600 (size 1024):
comm "softirq", pid 0, jiffies 4384527763 (age 2770.629s)
hex dump (first 32 bytes):
00 80 23 5d 0b 88 ff ff 00 00 00 00 00 00 00 00 ..#]............
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000d8a19b9d>] __alloc_skb+0xc9/0x520
[<000000001709b32f>] skb_segment+0x8c8/0x3710
[<00000000c7b9bb88>] tcp_gso_segment+0x331/0x1830
[<00000000c921cba1>] inet_gso_segment+0x476/0x1370
[<000000008b762dd4>] skb_mac_gso_segment+0x1f9/0x510
[<000000002182660a>] __skb_gso_segment+0x1dd/0x620
[<00000000412651b9>] netem_enqueue+0x1536/0x2590 [sch_netem]
[<0000000005d3b2a9>] __dev_queue_xmit+0x1167/0x2120
[<00000000fc5f7327>] ip_finish_output2+0x998/0xf00
[<00000000d309e9d3>] ip_output+0x1aa/0x2c0
[<000000007ecbd3a4>] tcp_transmit_skb+0x18db/0x3670
[<0000000042d2a45f>] tcp_write_xmit+0x4d4/0x58c0
[<0000000056a44199>] tcp_tasklet_func+0x3d9/0x540
[<0000000013d06d02>] tasklet_action+0x1ca/0x250
[<00000000fcde0b8b>] __do_softirq+0x1b4/0x5a3
[<00000000e7ed027c>] irq_exit+0x1e2/0x210
Fix it by adding the rest of the segments, if any, to skb 'to_free'
list. Add new __qdisc_drop_all() and qdisc_drop_all() functions
because they can be useful in the future if we need to drop segmented
GSO packets in other places.
Fixes: 6071bd1aa13e ("netem: Segment GSO packets on enqueue")
Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
v3: use skb->prev to find the tail of the list. skb->prev can be NULL
for not segmented skbs, so check it too.
v2: add new __qdisc_drop_all() and qdisc_drop_all(), and use
qdisc_drop_all() in sch_netem.
include/net/sch_generic.h | 19 +++++++++++++++++++
net/sched/sch_netem.c | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index e2ab136..2092d33 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -824,6 +824,16 @@ static inline void __qdisc_drop(struct sk_buff *skb, struct sk_buff **to_free)
*to_free = skb;
}
+static inline void __qdisc_drop_all(struct sk_buff *skb,
+ struct sk_buff **to_free)
+{
+ if (skb->prev)
+ skb->prev->next = *to_free;
+ else
+ skb->next = *to_free;
+ *to_free = skb;
+}
+
static inline unsigned int __qdisc_queue_drop_head(struct Qdisc *sch,
struct qdisc_skb_head *qh,
struct sk_buff **to_free)
@@ -956,6 +966,15 @@ static inline int qdisc_drop(struct sk_buff *skb, struct Qdisc *sch,
return NET_XMIT_DROP;
}
+static inline int qdisc_drop_all(struct sk_buff *skb, struct Qdisc *sch,
+ struct sk_buff **to_free)
+{
+ __qdisc_drop_all(skb, to_free);
+ qdisc_qstats_drop(sch);
+
+ return NET_XMIT_DROP;
+}
+
/* Length to Time (L2T) lookup in a qdisc_rate_table, to determine how
long it will take to send a packet given its size.
*/
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 7c179ad..7d6801f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -509,7 +509,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
if (unlikely(sch->q.qlen >= sch->limit))
- return qdisc_drop(skb, sch, to_free);
+ return qdisc_drop_all(skb, sch, to_free);
qdisc_qstats_backlog_inc(sch, skb);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net v3] sch_netem: fix skb leak in netem_enqueue()
2018-03-05 17:52 [PATCH net v3] sch_netem: fix skb leak in netem_enqueue() Alexey Kodanev
@ 2018-03-05 23:43 ` Neil Horman
2018-03-07 16:18 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Neil Horman @ 2018-03-05 23:43 UTC (permalink / raw)
To: Alexey Kodanev; +Cc: netdev, Stephen Hemminger, Eric Dumazet, David Miller
On Mon, Mar 05, 2018 at 08:52:54PM +0300, Alexey Kodanev wrote:
> When we exceed current packets limit and we have more than one
> segment in the list returned by skb_gso_segment(), netem drops
> only the first one, skipping the rest, hence kmemleak reports:
>
> unreferenced object 0xffff880b5d23b600 (size 1024):
> comm "softirq", pid 0, jiffies 4384527763 (age 2770.629s)
> hex dump (first 32 bytes):
> 00 80 23 5d 0b 88 ff ff 00 00 00 00 00 00 00 00 ..#]............
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<00000000d8a19b9d>] __alloc_skb+0xc9/0x520
> [<000000001709b32f>] skb_segment+0x8c8/0x3710
> [<00000000c7b9bb88>] tcp_gso_segment+0x331/0x1830
> [<00000000c921cba1>] inet_gso_segment+0x476/0x1370
> [<000000008b762dd4>] skb_mac_gso_segment+0x1f9/0x510
> [<000000002182660a>] __skb_gso_segment+0x1dd/0x620
> [<00000000412651b9>] netem_enqueue+0x1536/0x2590 [sch_netem]
> [<0000000005d3b2a9>] __dev_queue_xmit+0x1167/0x2120
> [<00000000fc5f7327>] ip_finish_output2+0x998/0xf00
> [<00000000d309e9d3>] ip_output+0x1aa/0x2c0
> [<000000007ecbd3a4>] tcp_transmit_skb+0x18db/0x3670
> [<0000000042d2a45f>] tcp_write_xmit+0x4d4/0x58c0
> [<0000000056a44199>] tcp_tasklet_func+0x3d9/0x540
> [<0000000013d06d02>] tasklet_action+0x1ca/0x250
> [<00000000fcde0b8b>] __do_softirq+0x1b4/0x5a3
> [<00000000e7ed027c>] irq_exit+0x1e2/0x210
>
> Fix it by adding the rest of the segments, if any, to skb 'to_free'
> list. Add new __qdisc_drop_all() and qdisc_drop_all() functions
> because they can be useful in the future if we need to drop segmented
> GSO packets in other places.
>
> Fixes: 6071bd1aa13e ("netem: Segment GSO packets on enqueue")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> ---
>
> v3: use skb->prev to find the tail of the list. skb->prev can be NULL
> for not segmented skbs, so check it too.
>
> v2: add new __qdisc_drop_all() and qdisc_drop_all(), and use
> qdisc_drop_all() in sch_netem.
>
>
> include/net/sch_generic.h | 19 +++++++++++++++++++
> net/sched/sch_netem.c | 2 +-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index e2ab136..2092d33 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -824,6 +824,16 @@ static inline void __qdisc_drop(struct sk_buff *skb, struct sk_buff **to_free)
> *to_free = skb;
> }
>
> +static inline void __qdisc_drop_all(struct sk_buff *skb,
> + struct sk_buff **to_free)
> +{
> + if (skb->prev)
> + skb->prev->next = *to_free;
> + else
> + skb->next = *to_free;
> + *to_free = skb;
> +}
> +
> static inline unsigned int __qdisc_queue_drop_head(struct Qdisc *sch,
> struct qdisc_skb_head *qh,
> struct sk_buff **to_free)
> @@ -956,6 +966,15 @@ static inline int qdisc_drop(struct sk_buff *skb, struct Qdisc *sch,
> return NET_XMIT_DROP;
> }
>
> +static inline int qdisc_drop_all(struct sk_buff *skb, struct Qdisc *sch,
> + struct sk_buff **to_free)
> +{
> + __qdisc_drop_all(skb, to_free);
> + qdisc_qstats_drop(sch);
> +
> + return NET_XMIT_DROP;
> +}
> +
> /* Length to Time (L2T) lookup in a qdisc_rate_table, to determine how
> long it will take to send a packet given its size.
> */
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 7c179ad..7d6801f 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -509,7 +509,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> }
>
> if (unlikely(sch->q.qlen >= sch->limit))
> - return qdisc_drop(skb, sch, to_free);
> + return qdisc_drop_all(skb, sch, to_free);
>
> qdisc_qstats_backlog_inc(sch, skb);
>
> --
> 1.8.3.1
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net v3] sch_netem: fix skb leak in netem_enqueue()
2018-03-05 17:52 [PATCH net v3] sch_netem: fix skb leak in netem_enqueue() Alexey Kodanev
2018-03-05 23:43 ` Neil Horman
@ 2018-03-07 16:18 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2018-03-07 16:18 UTC (permalink / raw)
To: alexey.kodanev; +Cc: netdev, nhorman, stephen, edumazet
From: Alexey Kodanev <alexey.kodanev@oracle.com>
Date: Mon, 5 Mar 2018 20:52:54 +0300
> When we exceed current packets limit and we have more than one
> segment in the list returned by skb_gso_segment(), netem drops
> only the first one, skipping the rest, hence kmemleak reports:
...
> Fix it by adding the rest of the segments, if any, to skb 'to_free'
> list. Add new __qdisc_drop_all() and qdisc_drop_all() functions
> because they can be useful in the future if we need to drop segmented
> GSO packets in other places.
>
> Fixes: 6071bd1aa13e ("netem: Segment GSO packets on enqueue")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> ---
>
> v3: use skb->prev to find the tail of the list. skb->prev can be NULL
> for not segmented skbs, so check it too.
>
> v2: add new __qdisc_drop_all() and qdisc_drop_all(), and use
> qdisc_drop_all() in sch_netem.
Applied and queued up for -stable, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-07 16:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-05 17:52 [PATCH net v3] sch_netem: fix skb leak in netem_enqueue() Alexey Kodanev
2018-03-05 23:43 ` Neil Horman
2018-03-07 16:18 ` David Miller
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).