diff mbox series

net: sched: Fix potential infinite loop

Message ID 20210407163808.499027-1-colin.king@canonical.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: sched: Fix potential infinite loop | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers fail 1 blamed authors not CCed: vsaicharan1998@gmail.com; 2 maintainers not CCed: vsaicharan1998@gmail.com kuba@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Colin King April 7, 2021, 4:38 p.m. UTC 
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(-)

Comments

Davide Caratti April 7, 2021, 8:20 p.m. UTC | #1 
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!
diff mbox series

Patch

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);