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=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 A966CC07E9C for ; Wed, 7 Jul 2021 12:50:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3E1B961CBD for ; Wed, 7 Jul 2021 12:50:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3E1B961CBD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 17E8B6B0036; Wed, 7 Jul 2021 08:50:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 155BB6B005D; Wed, 7 Jul 2021 08:50:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 01D906B006C; Wed, 7 Jul 2021 08:50:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0068.hostedemail.com [216.40.44.68]) by kanga.kvack.org (Postfix) with ESMTP id D09906B0036 for ; Wed, 7 Jul 2021 08:50:10 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 30286824999B for ; Wed, 7 Jul 2021 12:50:10 +0000 (UTC) X-FDA: 78335774580.01.A88C433 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf21.hostedemail.com (Postfix) with ESMTP id A1FA6D0025C2 for ; Wed, 7 Jul 2021 12:50:09 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 887AC61CBA; Wed, 7 Jul 2021 12:50:06 +0000 (UTC) Date: Wed, 7 Jul 2021 13:50:03 +0100 From: Catalin Marinas To: Robin Murphy Cc: Chen Huang , Al Viro , Matthew Wilcox , Christoph Hellwig , Mark Rutland , Andrew Morton , Stephen Rothwell , Randy Dunlap , Will Deacon , Linux ARM , linux-mm , open list Subject: Re: [BUG] arm64: an infinite loop in generic_perform_write() Message-ID: <20210707125003.GA24397@arm.com> References: <27fbb8c1-2a65-738f-6bec-13f450395ab7@arm.com> <20210624185554.GC25097@arm.com> <20210625103905.GA20835@arm.com> <7f14271a-9b2f-1afc-3caf-c4e5b36efa73@arm.com> <20210706175052.GD15218@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Authentication-Results: imf21.hostedemail.com; dkim=none; spf=pass (imf21.hostedemail.com: domain of cmarinas@kernel.org designates 198.145.29.99 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: A1FA6D0025C2 X-Stat-Signature: ohic4er3h31xrrraadoku7zqpoi9c8pq X-HE-Tag: 1625662209-474676 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 Tue, Jul 06, 2021 at 08:15:47PM +0100, Robin Murphy wrote: > On 2021-07-06 18:50, Catalin Marinas wrote: > > On Mon, Jun 28, 2021 at 05:22:30PM +0100, Robin Murphy wrote: > > > @@ -62,6 +64,12 @@ EXPORT_SYMBOL(__arch_copy_to_user) > > > .section .fixup,"ax" > > > .align 2 > > > +9997: cmp dst, dstin > > > + b.ne 9998f > > > + // Before being absolutely sure we couldn't copy anything, try harder > > > + ldrb tmp1w, [srcin] > > > +USER(9998f, sttrb tmp1w, [dstin]) > > > + add dst, dstin, #1 > > > 9998: sub x0, end, dst // bytes not copied > > > ret > > > .previous > > > > I think it's worth doing the copy_to_user() fallback in a loop until it > > faults or hits the end of the buffer. This would solve the problem we > > currently have with writing more bytes than actually reported. The > > copy_from_user() is not necessary, a byte would suffice. > > The thing is, we don't really have that problem since the set_fs cleanup > removed IMP-DEF STP behaviour from the picture - even with the current mess > we could perfectly well know which of the two STTRs faulted if we just put a > little more effort in. I think there are some corner cases: STTR across a page boundary, faulting on the second page. The architecture allows some data to be written (or not) in the first page, so we'd under-report if we use the destination update. If we use the fault address it's even worse as we may over-report in case the instruction did not write anything. > But yuck... If you think the potential under-reporting is worth fixing right > now, rather than just letting it disappear in a future rewrite, then I'd > still rather do it by passing the actual fault address to the current > copy_to_user fixup. After some more digging in the ARM ARM, I don't think that's fixable by using the actual fault address. B2.2.1 and D1.13.5 in version G.a (thanks to Will for digging them out) mean that for an interrupted store (exception, interrupt), any bytes stored by the instruction become UNKNOWN. In practice, this means left unchanged or written. So I think a byte-wise write loop is the only chance we have at a more precise reporting, unless we change the loops to align the writes. > A retry loop could still technically under-report if the > page disappears (or tag changes) between faulting on the second word of a > pair and retrying from the first, so we'd want to pin the initial fault down > to a single access anyway. All the loop would achieve after that is > potentially fill in an extra 1-7 bytes right up to the offending page/tag > boundary for the sake of being nice, which I remain unconvinced is worth the > bother :) There is indeed the risk of a race but we can blame the user for concurrently changing the permissions or tag. The kernel wouldn't normally do this. -- Catalin