Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 28 Oct 2020 11:19:59 -0700
From:   Stephen Hemminger <stephen@networkplumber.org>
To:     Aleksandr Nogikh <aleksandrnogikh@gmail.com>
Cc:     jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
        davem@davemloft.net, kuba@kernel.org, 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: Re: [PATCH v2] netem: fix zero division in tabledist
Message-ID: <20201028111959.6ed6d2c2@hermes.local>
In-Reply-To: <20201028170731.1383332-1-aleksandrnogikh@gmail.com>
References: <20201028170731.1383332-1-aleksandrnogikh@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 28 Oct 2020 17:07:31 +0000
Aleksandr Nogikh <aleksandrnogikh@gmail.com> wrote:

> From: Aleksandr Nogikh <nogikh@google.com>
> 
> Currently it is possible to craft a special netlink RTM_NEWQDISC
> command that can result in jitter being equal to 0x80000000. It is
> enough to set the 32 bit jitter to 0x02000000 (it will later be
> multiplied by 2^6) or just set the 64 bit jitter via
> TCA_NETEM_JITTER64. This causes an overflow during the generation of
> uniformly distributed 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 keeping the value of jitter within the range that can
> be adequately handled by tabledist() - [0;INT_MAX]. As negative std
> deviation makes no sense, take the absolute value of the passed value
> and cap it at INT_MAX. Inside tabledist(), switch to unsigned 32 bit
> arithmetic in order to prevent overflows.
> 
> Signed-off-by: Aleksandr Nogikh <nogikh@google.com>
> Reported-by: syzbot+ec762a6342ad0d3c0d8f@syzkaller.appspotmail.com

Acked-by: Stephen Hemminger <stephen@networkplumber.org>