netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aleksandr Nogikh <a.nogikh@yandex.ru>
To: stephen@networkplumber.org, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
	kuba@kernel.org
Cc: andreyknvl@google.com, dvyukov@google.com, elver@google.com,
	rdunlap@infradead.org, dave.taht@gmail.com, edumazet@google.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Aleksandr Nogikh <nogikh@google.com>,
	syzbot+ec762a6342ad0d3c0d8f@syzkaller.appspotmail.com
Subject: [PATCH] netem: prevent division by zero in tabledist
Date: Fri, 16 Oct 2020 12:10:07 +0000	[thread overview]
Message-ID: <20201016121007.2378114-1-a.nogikh@yandex.ru> (raw)

From: Aleksandr Nogikh <nogikh@google.com>

Currently it is possible to craft a special netlink RTM_NEWQDISC
command that result in jitter being equal to 0x80000000. It is enough
to set 32 bit jitter to 0x02000000 (it will later be multiplied by
2^6) or set 64 bit jitter via TCA_NETEM_JITTER64. This causes an
overflow during the generation of uniformly districuted numbers in
tabledist, which in turn leads to division by zero (sigma != 0, but
sigma * 2 is 0).

The related fragment of code needs 32-bit division - see commit
9b0ed89 ("netem: remove unnecessary 64 bit modulus"), so
switching to 64 bit is not an option.

Fix the issue by preventing 32 bit integer overflows in
tabledist. Also, instead of truncating s64 integer to s32, truncate it
to u32, as negative standard deviation does not make sense anyway.

Reported-by: syzbot+ec762a6342ad0d3c0d8f@syzkaller.appspotmail.com
Signed-off-by: Aleksandr Nogikh <nogikh@google.com>
---
 net/sched/sch_netem.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 84f82771cdf5..d8b0bf1b5346 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -315,7 +315,7 @@ static bool loss_event(struct netem_sched_data *q)
  * std deviation sigma.  Uses table lookup to approximate the desired
  * distribution, and a uniformly-distributed pseudo-random source.
  */
-static s64 tabledist(s64 mu, s32 sigma,
+static s64 tabledist(s64 mu, u32 sigma,
 		     struct crndstate *state,
 		     const struct disttable *dist)
 {
@@ -329,8 +329,14 @@ static s64 tabledist(s64 mu, s32 sigma,
 	rnd = get_crandom(state);
 
 	/* default uniform distribution */
-	if (dist == NULL)
+	if (!dist) {
+		/* Sigma is too big to perform 32 bit division.
+		 * Use the widest possible deviation.
+		 */
+		if ((u64)sigma * 2ULL >= U32_MAX)
+			return mu + rnd - U32_MAX / 2;
 		return ((rnd % (2 * sigma)) + mu) - sigma;
+	}
 
 	t = dist->table[rnd % dist->size];
 	x = (sigma % NETEM_DIST_SCALE) * t;
@@ -533,7 +539,10 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		u64 now;
 		s64 delay;
 
-		delay = tabledist(q->latency, q->jitter,
+		/* tabledist is unable to handle 64 bit jitters yet, so we adjust it beforehand */
+		u32 constrained_jitter = q->jitter > 0 ? min_t(u32, q->jitter, U32_MAX) : 0;
+
+		delay = tabledist(q->latency, constrained_jitter,
 				  &q->delay_cor, q->delay_dist);
 
 		now = ktime_get_ns();

base-commit: bbf5c979011a099af5dc76498918ed7df445635b
-- 
2.29.0.rc1.297.gfa9743e501-goog


             reply	other threads:[~2020-10-16 12:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-16 12:10 Aleksandr Nogikh [this message]
2020-10-16 15:53 ` [PATCH] netem: prevent division by zero in tabledist Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201016121007.2378114-1-a.nogikh@yandex.ru \
    --to=a.nogikh@yandex.ru \
    --cc=andreyknvl@google.com \
    --cc=dave.taht@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=elver@google.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nogikh@google.com \
    --cc=rdunlap@infradead.org \
    --cc=stephen@networkplumber.org \
    --cc=syzbot+ec762a6342ad0d3c0d8f@syzkaller.appspotmail.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).