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

From: zijun_hu
Date: Fri Sep 23 2016 - 11:41:48 EST

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@xxxxxxx>
>> 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@xxxxxxx>
> 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