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]
Message-ID: <87pn61efcs.fsf@nanos.tec.linutronix.de>
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



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ