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]

Date:   Fri, 23 Sep 2016 23:41:33 +0800
From:   zijun_hu <zijun_hu@...o.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     zijun_hu@....com, Andrew Morton <akpm@...ux-foundation.org>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org, 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

On 2016/9/23 22:42, Tejun Heo wrote:
> 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.
> 
thanks for your reply firstly
1. ioremap_page_range() is not a kernel internal function
2. sanity check "BUG_ON(addr >= end)" have existed already, but don't check enough
3. are there any obvious hint for rang parameter requirements but BUG_ON(addr >= end)
4. if range which seems right but wrong really is used such as mapping 
   virtual range [0x80000800, 0x80007800) to physical area[0x20000800, 0x20007800)
   what actions should we take? warning message and trying to finish user request
   or panic kernel or hang system in endless loop or return -EINVAL？
   how to help user find their problem?
5. if both boundary of the range are aligned to page, ioremap_page_range() works well
   otherwise endless loop maybe happens