linux-kernel - Re: [PATCH v2 1/2] ras: fix an off-by-one error in __find

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: <20190420190442.GF29704@zn.tnic>
Date:   Sat, 20 Apr 2019 21:04:42 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, linux-edac@...r.kernel.org,
        Tony Luck <tony.luck@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2 1/2] ras: fix an off-by-one error in __find_elem()

On Sat, Apr 20, 2019 at 11:25:43AM -0700, Cong Wang wrote:
> If you want to go that far, you can choose to use lib/bsearch.c too in
> case you want to reinvent the wheel.

Well, that doesn't give me the @to functionality which points to the
slot where the new element should be inserted, when the search was
unsuccessful.

> What's your point here?

My point is to fix it properly. Obviously.

> You know my fix is targeted for -stable,

Well, first you sent me this:

https://lkml.kernel.org/r/20190416012001.5338-1-xiyou.wangcong@gmail.com

then this:

https://lkml.kernel.org/r/20190416213351.28999-1-xiyou.wangcong@gmail.com

Tony liked this second version more and if you look at the final result of mine:

	int min = 0, max = ca->n - 1;

	...

                if (this_pfn < pfn)
                        min = i + 1;
                else if (this_pfn > pfn)
                        max = i - 1;
                else if (this_pfn == pfn) {
                        if (to)
                                *to = i;
                        return i;
                }

it has basically *both*: the correct [min:max] range *and* the return of
ithe ndex when found. But the algorithm this time is the correct one.

> I doubt your 83-line change could fit for -stable.

My 83-line change has debug output only for experimentation. It will,
*of* *course* be removed before committing it upstream. That's why I
called it "a conglomerate patch" and I said "It has some debug output
for easier debugging, that will be removed in the final version, of
course." I guess you didn't read that either.

And the sanity_check() piece will be a separate patch, of course.

In the end the diffstat will be 30-40 lines max.

> Feel free to drop my patch to favor yours. I am really tired.

Suit yourself. Thanks for the reporting.

> Good luck with that!

Ditto.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.