From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4333DC433DB for ; Wed, 23 Dec 2020 01:51:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id BC73122583 for ; Wed, 23 Dec 2020 01:51:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BC73122583 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DA1A88D0008; Tue, 22 Dec 2020 20:51:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D50C28D0001; Tue, 22 Dec 2020 20:51:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C1A8D8D0008; Tue, 22 Dec 2020 20:51:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0061.hostedemail.com [216.40.44.61]) by kanga.kvack.org (Postfix) with ESMTP id AA0948D0001 for ; Tue, 22 Dec 2020 20:51:00 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 6B85E8249980 for ; Wed, 23 Dec 2020 01:51:00 +0000 (UTC) X-FDA: 77622868680.15.test18_0c1714827464 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id 45E4E1814B0C7 for ; Wed, 23 Dec 2020 01:51:00 +0000 (UTC) X-HE-Tag: test18_0c1714827464 X-Filterd-Recvd-Size: 5290 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf30.hostedemail.com (Postfix) with ESMTP for ; Wed, 23 Dec 2020 01:50:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To:Subject:Sender :Reply-To:Content-ID:Content-Description; bh=Y/ki+pwiajt1JGckMLPHR2xc4WDa7IQDL+LHZUzD3iA=; b=bS5HJD18j31Q+5eh0KZPDYrfP3 HSXt662IfbFyfBOCC+BEL0uRp8jyfjaojciTHRc9uyP3pnB6vIp2bGYVkfH5NsccYlfix2LrYHLsn XLj8umTznJGKEaDTGMZN36n2lEkclfTB5M0+Fn+hE3KQhs0xVBC2IzxD+wBzjRA1yCwvIP8MdYmwY dOEirZbCqKt9IRVRu8PXfxpTz1hpV9oJ5AL612jdWfJ+xzjA+V9SP5Iwu6YTg8D73oDAGuSrMqQ1H F5OOMqpGj0/X8iIPISOvUCWxcykHEehyA9uMJk8ULbK3vNTc193DhxViBDg/EFM6xZGAb8mED7JOm 6eSNXE6Q==; Received: from [2601:1c0:6280:3f0::64ea] by casper.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat Linux)) id 1krtIn-0006V8-92; Wed, 23 Dec 2020 01:50:57 +0000 Subject: Re: [RFC PATCH 2/2] mm: readahead: handle LARGE input to get_init_ra_size() To: Andrew Morton Cc: linux-kernel@vger.kernel.org, Jens Axboe , =?UTF-8?Q?Toralf_F=c3=b6rster?= , linux-mm@kvack.org References: <20201220211051.1416-1-rdunlap@infradead.org> <20201222173533.c9e28416835d7487b0e28cda@linux-foundation.org> From: Randy Dunlap Message-ID: <6a595671-20a8-e63f-f3ea-f4749a574efa@infradead.org> Date: Tue, 22 Dec 2020 17:50:52 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <20201222173533.c9e28416835d7487b0e28cda@linux-foundation.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 12/22/20 5:35 PM, Andrew Morton wrote: > On Sun, 20 Dec 2020 13:10:51 -0800 Randy Dunlap wrote: > >> Add a test to detect if the input ra request size has its high order >> bit set (is negative when tested as a signed long). This would be a >> really Huge readahead. >> >> If so, WARN() with the value and a stack trace so that we can see >> where this is happening and then make further corrections later. >> Then adjust the size value so that it is not so Huge (although >> this may not be needed). > > What motivates this change? Is there any reason to think this can > happen? Spotted in the wild: mr-fox kernel: [ 1974.206977] UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13 mr-fox kernel: [ 1974.206980] shift exponent 64 is too large for 64-bit type 'long unsigned int' Original report: https://lore.kernel.org/lkml/c6e5eb81-680f-dd5c-8a81-62041a5ce50c@gmx.de/ Willy suggested that get_init_ra_size() was being called with a size of 0, which would cause this (instead of some Huge value), so I made a follow-up patch that only checks size for 0 and if 0, defaults it to 32 (pages). --- mm/readahead.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) --- linux-5.10.1.orig/mm/readahead.c +++ linux-5.10.1/mm/readahead.c @@ -310,7 +310,11 @@ void force_page_cache_ra(struct readahea */ static unsigned long get_init_ra_size(unsigned long size, unsigned long max) { - unsigned long newsize = roundup_pow_of_two(size); + unsigned long newsize; + + if (!size) + size = 32; + newsize = roundup_pow_of_two(size); if (newsize <= max / 32) newsize = newsize * 4; Toralf has only seen this problem one time. > Also, everything in there *should* be unsigned, because a negative > readahead is semantically nonsensical. Is our handling of this > inherently unsigned quantity incorrect somewhere? > >> --- linux-5.10.1.orig/mm/readahead.c >> +++ linux-5.10.1/mm/readahead.c >> >> ... >> >> @@ -303,14 +304,21 @@ void force_page_cache_ra(struct readahea >> } >> >> /* >> - * Set the initial window size, round to next power of 2 and square >> + * Set the initial window size, round to next power of 2 >> * for small size, x 4 for medium, and x 2 for large >> * for 128k (32 page) max ra >> * 1-8 page = 32k initial, > 8 page = 128k initial >> */ >> static unsigned long get_init_ra_size(unsigned long size, unsigned long max) >> { >> - unsigned long newsize = roundup_pow_of_two(size); >> + unsigned long newsize; >> + >> + if ((signed long)size < 0) { /* high bit is set: ultra-large ra req */ >> + WARN_ONCE(1, "%s: size=0x%lx\n", __func__, size); >> + size = -size; /* really only need to flip the high/sign bit */ >> + } >> + >> + newsize = roundup_pow_of_two(size); > > Is there any way in which userspace can deliberately trigger warning? > Via sys_readadhead() or procfs tuning or whatever? > > I guess that permitting a user-triggerable WARN_ONCE() isn't a huuuuge > problem - it isn't a DoS if it only triggers a single time. It does > permit the malicious user to disable future valid warnings, but I don't > see what incentive there would be for this. But still, it seems > desirable to avoid it. Sure. I think that we can drop RFC patches 1/2 and 2/2 and just consider the other one above. -- ~Randy