* [PATCH] net: sched: Fix potential infinite loop
@ 2021-04-07 16:38 Colin King
2021-04-07 20:20 ` Davide Caratti
0 siblings, 1 reply; 2+ messages in thread
From: Colin King @ 2021-04-07 16:38 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller,
Gautam Ramakrishnan, Sachin D . Patil, Mohit Bhasi, Leslie Monis,
Mohit P . Tahiliani, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The for-loop iterates with a u16 loop counter idx and compares this
with the loop upper limit of q->flows_cnt that is a u32 type.
There is a potential infinite loop if q->flows_cnt is larger than
the u8 loop counter. Fix this by making the loop counter the same
type as q->flows_cnt.
Addresses-Coverity: ("Infinite loop")
Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/sched/sch_fq_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 949163fe68af..00563e137ecb 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -367,7 +367,7 @@ static void fq_pie_timer(struct timer_list *t)
struct fq_pie_sched_data *q = from_timer(q, t, adapt_timer);
struct Qdisc *sch = q->sch;
spinlock_t *root_lock; /* to lock qdisc for probability calculations */
- u16 idx;
+ u32 idx;
root_lock = qdisc_lock(qdisc_root_sleeping(sch));
spin_lock(root_lock);
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] net: sched: Fix potential infinite loop
2021-04-07 16:38 [PATCH] net: sched: Fix potential infinite loop Colin King
@ 2021-04-07 20:20 ` Davide Caratti
0 siblings, 0 replies; 2+ messages in thread
From: Davide Caratti @ 2021-04-07 20:20 UTC (permalink / raw)
To: Colin King, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
David S . Miller, Gautam Ramakrishnan, Sachin D . Patil,
Mohit Bhasi, Leslie Monis, Mohit P . Tahiliani, netdev
Cc: kernel-janitors, linux-kernel
hello Colin, and thanks for your patch!
On Wed, 2021-04-07 at 17:38 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The for-loop iterates with a u16 loop counter idx and compares this
> with the loop upper limit of q->flows_cnt that is a u32 type.
the value of 'flows_cnt' has 65535 as an upper bound in the ->init()
function, so it should be safe to use an u16 for 'idx'. (BTW, the
infinite loop loop was a real thing, see [1] :) ).
> There is a potential infinite loop if q->flows_cnt is larger than
> the u8 loop counter.
(u16 loop counter, IIUC)
> Fix this by making the loop counter the same
> type as q->flows_cnt.
the same 'for' loop is in fq_pie_init() and fq_pie_reset(): so, in my
opinion just changing fq_pie_timer() to fix an infinite loop is not very
useful: 'idx' is also used as an index for q->flows[], that's allocated
in [2]. Maybe (but I might be wrong) just allowing bigger values might
potentially cause other covscan warnings. WDYT?
> Addresses-Coverity: ("Infinite loop")
> Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
thanks!
--
davide
[1] https://lore.kernel.org/netdev/416eb03a8ca70b5dfb5e882e2752b7fc13c42f92.1590537338.git.dcaratti@redhat.com/
[2] https://elixir.bootlin.com/linux/latest/source/net/sched/sch_fq_pie.c#L417
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-04-07 20:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 16:38 [PATCH] net: sched: Fix potential infinite loop Colin King
2021-04-07 20:20 ` Davide Caratti
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).