linux-kernel - Re: [PATCH 1/1] lib/ioremap.c: avoid endless loop under ioremapping page unaligned ranges

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: <20160923144202.GA31387@htj.duckdns.org>
Date:   Fri, 23 Sep 2016 10:42:02 -0400
From:   Tejun Heo <tj@...nel.org>
To:     zijun_hu <zijun_hu@...o.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, zijun_hu@....com, mingo@...nel.org,
        rientjes@...gle.com, iamjoonsoo.kim@....com,
        mgorman@...hsingularity.net
Subject: Re: [PATCH 1/1] lib/ioremap.c: avoid endless loop under ioremapping
 page unaligned ranges

Hello,

On Wed, Sep 21, 2016 at 12:19:53PM +0800, zijun_hu wrote:
> From: zijun_hu <zijun_hu@....com>
> 
> endless loop maybe happen if either of parameter addr and end is not
> page aligned for kernel API function ioremap_page_range()
> 
> in order to fix this issue and alert improper range parameters to user
> WARN_ON() checkup and rounding down range lower boundary are performed
> firstly, loop end condition within ioremap_pte_range() is optimized due
> to lack of relevant macro pte_addr_end()
> 
> Signed-off-by: zijun_hu <zijun_hu@....com>

Unfortunately, I can't see what the points are in this series of
patches.  Most seem to be gratuitous changes which don't address real
issues or improve anything.  "I looked at the code and realized that,
if the input were wrong, the function would misbehave" isn't good
enough a reason.  What's next?  Are we gonna harden all pointer taking
functions too?

For internal functions, we don't by default do input sanitization /
sanity check.  There sure are cases where doing so is beneficial but
reading a random function and thinking "oh this combo of parameters
can make it go bonkers" isn't the right approach for it.  We end up
with cruft and code changes which nobody needed in the first place and
can easily introduce actual real bugs in the process.

It'd be an a lot more productive use of time and effort for everyone
involved if the work is around actual issues.

Thanks.

-- 
tejun