From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966947AbcIWPlp (ORCPT ); Fri, 23 Sep 2016 11:41:45 -0400 Received: from sender153-mail.zoho.com ([74.201.84.153]:25440 "EHLO sender153-mail.zoho.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965902AbcIWPlo (ORCPT ); Fri, 23 Sep 2016 11:41:44 -0400 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=zapps768; d=zoho.com; h=subject:to:references:cc:from:message-id:date:user-agent:mime-version:in-reply-to:content-type; b=pKNxgGLBdD97r3j5pLydubJwJEWveQcvDZZAGBeGhunXgKIzwAIU7xpD0dc9HMFTuCV4XeCxQql3 I/n40MuDySgtYFKCGMZvXUdoPuDOYsKTjTbECb8yZoT8QCaGOktp Subject: Re: [PATCH 1/1] lib/ioremap.c: avoid endless loop under ioremapping page unaligned ranges To: Tejun Heo References: <57E20A69.5010206@zoho.com> <20160923144202.GA31387@htj.duckdns.org> Cc: zijun_hu@htc.com, Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, mingo@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, mgorman@techsingularity.net From: zijun_hu Message-ID: <238b0d3e-2e6b-7f73-8168-d21517e862bb@zoho.com> Date: Fri, 23 Sep 2016 23:41:33 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160923144202.GA31387@htj.duckdns.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> >> 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 > > 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