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

[zijun_hu]

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