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

From: Tejun Heo
Date: Fri Sep 23 2016 - 10:42:11 EST


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.

--
tejun