netdev - Re: [PATCH 4/7] Fix qdisc_watchdog_schedule_range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 02 Oct 2020 00:44:19 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Erez Geva <erez.geva.ext@...mens.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        Cong Wang <xiyou.wangcong@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>, Andrei Vagin <avagin@...il.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        Ingo Molnar <mingo@...nel.org>,
        John Stultz <john.stultz@...aro.org>,
        Michal Kubecek <mkubecek@...e.cz>,
        Oleg Nesterov <oleg@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Richard Cochran <richardcochran@...il.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Vladis Dronov <vdronov@...hat.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Frederic Weisbecker <frederic@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     Jesus Sanchez-Palencia <jesus.sanchez-palencia@...el.com>,
        Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        Vedang Patel <vedang.patel@...el.com>,
        Simon Sudler <simon.sudler@...mens.com>,
        Andreas Meisinger <andreas.meisinger@...mens.com>,
        Andreas Bucher <andreas.bucher@...mens.com>,
        Henning Schild <henning.schild@...mens.com>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        Andreas Zirkler <andreas.zirkler@...mens.com>,
        Ermin Sakic <ermin.sakic@...mens.com>,
        An Ninh Nguyen <anninh.nguyen@...mens.com>,
        Michael Saenger <michael.saenger@...mens.com>,
        Bernd Maehringer <bernd.maehringer@...mens.com>,
        Gisela Greinert <gisela.greinert@...mens.com>,
        Erez Geva <erez.geva.ext@...mens.com>,
        Erez Geva <ErezGeva2@...il.com>
Subject: Re: [PATCH 4/7] Fix qdisc_watchdog_schedule_range_ns range check

On Thu, Oct 01 2020 at 22:51, Erez Geva wrote:

Fixes should be at the beginning of a patch series and not be hidden
somewhere in the middle.

>    - As all parameters are unsigned.

This is not a sentence and this list style does not make that changelog
more readable.

>    - If 'expires' is bigger than 'last_expires' then the left expression
>      overflows.

This would be the most important information and should be clearly
spelled out as problem description at the very beginning of the change
log.

>    - It is better to use addition and check both ends of the range.

Is better? Either your change is correcting the problem or not. Just
better but incorrect does not cut it.

But let's look at the problem itself. The check is about:

    B <= A <= B + C

A, B, C are all unsigned. So if B > A then the result is false.

Now lets look at the implementation:

    if (A - B <= C)
    	return;

which works correctly due the wonders of unsigned math.

For B <= A the check is obviously correct.

If B > A then the result of the unsigned subtraction A - B is a very
large positive number which is pretty much guaranteed to be larger than
C, i.e. the result is false.

So while not immediately obvious, it's still correct.

Thanks,

        tglx