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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 512B5C001B0 for ; Thu, 10 Aug 2023 19:23:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D3D7E6B0072; Thu, 10 Aug 2023 15:23:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CED956B0078; Thu, 10 Aug 2023 15:23:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B8E876B007B; Thu, 10 Aug 2023 15:23:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id A74926B0072 for ; Thu, 10 Aug 2023 15:23:43 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 6C15CA0194 for ; Thu, 10 Aug 2023 19:23:43 +0000 (UTC) X-FDA: 81109169526.28.E762A42 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf19.hostedemail.com (Postfix) with ESMTP id 942511A000D for ; Thu, 10 Aug 2023 19:23:41 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=DAMSaiGf; spf=pass (imf19.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691695421; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=3sKCHgBg4mxifvM6Xa6BVMbAED3Wj/G0loImdmtFwEk=; b=jv3OENpVHNNJXDq5xohynRjL4ZimXPimWzirzKouICjmNIDIezsZx6395vPwVG39uyDYxJ FNpPV6lXb5iYOo+xj0AUkVHHg1h3PW4cd1fkbguNubuzG1oz4JaEO5SjLZo0JJf8Eyzqgc 7hOyBu7jxRKMGRBApXxm4Vu0UQph5Fc= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=DAMSaiGf; spf=pass (imf19.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691695421; a=rsa-sha256; cv=none; b=oPphzK9rl+t9i/g7qeC0KEszmG33DpxIUmbu8wJLd+bVEwKti5qc2fi4Pk/Xwwrz7DFG8g ai6gktSEHXWRE/ajcWkjJzTyq5HDbgxOuiW+7/Dp0qQeX95/0YgqDfs9JQeevg4NNqHY/e mlcPvadybRQ58Tnv/n+4No6VupUD01o= Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-99bdcade7fbso177025666b.1 for ; Thu, 10 Aug 2023 12:23:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691695420; x=1692300220; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3sKCHgBg4mxifvM6Xa6BVMbAED3Wj/G0loImdmtFwEk=; b=DAMSaiGfdsGZGv3h1zBYsjy/6kNTWZZ2jC6R+Gq/wb3WezwGLerdOYU1/NTBCz+nff Pb1fVwIQ8DEgjqYpuP2vO9hxYyRLTiemilsqRmjq2PDFCTFdJ/yIaguegGQFWGlwwOWk Ac5PwwJX5zbhqOPWWhUggVVx9vdPW9MaoYS3j2FitmTlfP5SZ2R0eSrF5vw/aa8YXX5n 8glBok4T8cHMKBS5lyCUvbayTRkq31XB+sSVzNthfLXdaraoZBAlD+JgX07rapqt71Xl LOp7ZHSIvjWH+hCMZZdI9kScmZgVnx9iS1UYtG7wIn2K5vKDOa9b/hL7m4GO+ErEPlUC PTmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691695420; x=1692300220; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3sKCHgBg4mxifvM6Xa6BVMbAED3Wj/G0loImdmtFwEk=; b=Ws5cKB79mcZYmGQr25LE+7YCS+5EleCkz2L5bKrWiMsyiFMMkC46J4Ngx/ljDDbLBH 5dJNW0qr1jtEY57ayn0lPNkO6M1rYAJtTy/h+HCNPAEGfDUWKWL0Jjn0cLM2NqSjQJul EbwkPJOhgJScEPAWeUsk4N+TXpT/S7+al32YoISDb0ykHDqEesiHZWR15CG8uIG25OTG bZyE6VFzIGnp7lwiNKtzmOMuG4n8ilZ7WHy0zEX6sfx3fOS/uGimWJuDsCknHd3Eogmu /i2zlYDAtjLOyc6egROmgLea7xegmRTws0st2A/gvz+SjSFWHNG1VmTVve2t9/onQJKa mw6g== X-Gm-Message-State: AOJu0Yw69TiK34dahMXv9BeC5Rov/R78h+01Yoj1MnoyM5vPyEQrgRiF mM66XHQ2cknzbt4b1Q60zG2fihCAO0enlgZt2tW73A== X-Google-Smtp-Source: AGHT+IHVLgL4hiCPcMn3G6OBC3NriMVoW4IgYUhiDT6aFvxlngYQ56xk71JfHlvM9pNfiVbx4Pnnj3sTDxi0Tkvfwhg= X-Received: by 2002:a17:906:5357:b0:997:e9a3:9c59 with SMTP id j23-20020a170906535700b00997e9a39c59mr2962918ejo.6.1691695419686; Thu, 10 Aug 2023 12:23:39 -0700 (PDT) MIME-Version: 1.0 References: <20230714182932.2608735-1-axelrasmussen@google.com> <8fbb5965-28f7-4e9a-ac04-1406ed8fc2d4@arm.com> <6a7bff41-259b-89f3-1a46-5b5b73d3fea1@redhat.com> In-Reply-To: <6a7bff41-259b-89f3-1a46-5b5b73d3fea1@redhat.com> From: Axel Rasmussen Date: Thu, 10 Aug 2023 12:23:03 -0700 Message-ID: Subject: Re: [PATCH mm-unstable fix] mm: userfaultfd: check for start + len overflow in validate_range: fix To: David Hildenbrand Cc: Ryan Roberts , Alexander Viro , Andrew Morton , Brian Geffon , Christian Brauner , Gaosheng Cui , Huang Ying , Hugh Dickins , James Houghton , Jiaqi Yan , Jonathan Corbet , Kefeng Wang , "Liam R. Howlett" , Miaohe Lin , Mike Kravetz , "Mike Rapoport (IBM)" , Muchun Song , Nadav Amit , Naoya Horiguchi , Peter Xu , Shuah Khan , Steven Barrett , Suleiman Souhlal , Suren Baghdasaryan , "T.J. Alumbaugh" , Yu Zhao , ZhangPeng , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, syzbot+42309678e0bc7b32f8e9@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 942511A000D X-Rspam-User: X-Stat-Signature: sdnbqgux41a43g6dydaygjso8meoeexc X-Rspamd-Server: rspam01 X-HE-Tag: 1691695421-127879 X-HE-Meta: U2FsdGVkX1+e50MduzZLMJnVxwxpaSxlN8wItNbZGygPF9L0AOnHMSbSY8ky7uWeRgmTOg7XWqiB2n9c2h3kfLF85te6Rs7r5rKzBamkfV3t5PxV7YH6ERYhPn6dRxVby4/jcZVEdMK31sRMFMUJDZM8ZCKnmECTJY+3yGSaUyzjba3P+stWSrQogYXtULxx40qZlv+gQcJP4yi+jvag3LsjnkIWmm5YG5xX0/W/5970xZ3aWtYf439upmOi2kJ8uM1eqs/mnXDofW9tYzhcqb6Chcrp0BmgYA1NGUV9V0yHue/9vEWKDA0hVcyITyw9tH2etVI/33OSdNzMjEXQc05sHM0nDvW0Mlhgm6Uj4nLA7AXWSCetuSE0zFRVqEgxiNUx4A6MzZl9/i6sz41ktDWMbr8PMVwoCoGz4PrQVEuJXYI8+z8zYTprGAGkIwe8z3G5ITUhuCYDv+HKF8Fn+bVFJ+p5B84eCm/LICzYaa2Gvo7cHOBHcCYqyYeIZu+Afrp8YMkP6KyIPo224alTzbEoahJ2MM+aVBpMC9QE6XUwk0J9ZGXW2BWm1uvuZ9m2OIDFDNQt9vO9P7y+b74RLy3np/6FJ+J9GKIhhTNlbJnvjF2tegm6U+n1MS+boi5uZmx+Yhbpq0YmsaatD1AMeNZD7lRPX7cMkW3HZWvjFnuuxR6K9LISOEe3Y27EhgsvpwFhrbwSMy65WObO/hiAR3BMVclTl5PEemF+SqnbCDS2JcrPU0IYOg/n4X//DLxSTCm9BXz3Mg/lnZHg4hK/ztlyTHhxZltvNXYrAYvP2J1srTDRovE+4LLfq/JBQFZFg9wniuKht8Qh9su7lnNf4ACm0UnUr5AvFEI7y0aXFGSD9jn6zt7uSpKbibnM3BLlYBqKawoXUqK5SP8Wh+bFw3SNggDbvZFeY6dluRZh3C057TXlvytFmvYFd0qmOSxr7usFDwkHe8vqp3VtjjF qDemzu67 KPxt691gjJbOREtVs9MeMvw4PX3vN0wF2hS2vHiCOmXFgHOF5hmsP7E3Or0S0y3cFCtBbWGuKBWukNx/l23CBp1I93QzRkzjJ/bO+yev9Z0MyXDf6VcPGJJsUn05+Xuv21xVY3fiDRZ2KFyOzCboL5Xkc20ZIRUi/qVFHMgwYqj4WhKnA5fP5OZmvt3QM6Q+zpSwLGsebm/x4nOqercKJGp1ApICFfFMSXwiHPIN2leuPe6OKd/5FTm0vqfTw9axr6UnMpvezct6oNQE3hkbuz9hgzIrmK0eT7O40sag9QuBL0i3e5C3QbLh2HQT9eBRB0Hkj8fOijrZr4JEME6kD14sODo+x64ytVKQUU55BooLe66txt8MlWTMUOPez/9EFTy6wK5eex9g7CcrUIKV68bsGW0Vniotwx9TUk2hWKeHAHKngLyN8VHVQGjONRleTzYwa/LS00bdV0LqIcqyh3pdcB2tvU5YWBpCRPrwXTQBCCz9Jr1qD8SsSRmRElzTJXniJPepj0ikq5z7row+osGayF0NvXIa7qmBNYpuYkAj4TfnEE3m4MHtTeKSvZn2T9dUtaUiw1E8aKP+1hDFqGht35sUKnK+Iikgq 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 Thu, Aug 10, 2023 at 9:49=E2=80=AFAM David Hildenbrand wrote: > > On 10.08.23 17:53, Ryan Roberts wrote: > > On 14/07/2023 19:29, Axel Rasmussen wrote: > >> This commit removed an extra check for zero-length ranges, and folded = it > >> into the common validate_range() helper used by all UFFD ioctls. > >> > >> It failed to notice though that UFFDIO_COPY *only* called validate_ran= ge > >> on the dst range, not the src range. So removing this check actually l= et > >> us proceed with zero-length source ranges, eventually hitting a BUG > >> further down in the call stack. > >> > >> The correct fix seems clear: call validate_range() on the src range to= o. > >> > >> Other ioctls are not affected by this, as they only have one range, no= t > >> two (src + dst). > >> > >> Reported-by: syzbot+42309678e0bc7b32f8e9@syzkaller.appspotmail.com > >> Closes: https://syzkaller.appspot.com/bug?extid=3D42309678e0bc7b32f8e9 > >> Signed-off-by: Axel Rasmussen > >> --- > >> fs/userfaultfd.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > >> index 53a7220c4679..36d233759233 100644 > >> --- a/fs/userfaultfd.c > >> +++ b/fs/userfaultfd.c > >> @@ -1759,6 +1759,9 @@ static int userfaultfd_copy(struct userfaultfd_c= tx *ctx, > >> sizeof(uffdio_copy)-sizeof(__s64))) > >> goto out; > >> > >> + ret =3D validate_range(ctx->mm, uffdio_copy.src, uffdio_copy.len)= ; > >> + if (ret) > >> + goto out; > >> ret =3D validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len)= ; > >> if (ret) > >> goto out; > > > > > > Hi Axel, > > > > I've just noticed that this patch, now in mm-unstable, regresses the mk= dirty mm > > selftest: > > > > # [INFO] detected THP size: 2048 KiB > > TAP version 13 > > 1..6 > > # [INFO] PTRACE write access > > ok 1 SIGSEGV generated, page not modified > > # [INFO] PTRACE write access to THP > > ok 2 SIGSEGV generated, page not modified > > # [INFO] Page migration > > ok 3 SIGSEGV generated, page not modified > > # [INFO] Page migration of THP > > ok 4 SIGSEGV generated, page not modified > > # [INFO] PTE-mapping a THP > > ok 5 SIGSEGV generated, page not modified > > # [INFO] UFFDIO_COPY > > not ok 6 UFFDIO_COPY failed > > Bail out! 1 out of 6 tests failed > > # Totals: pass:5 fail:1 xfail:0 xpass:0 skip:0 error:0 > > > > Whereas all 6 tests pass against v6.5-rc4. > > > > I'm afraid I don't know the test well and haven't looked at what the is= sue might > > be, but noticed and thought I should point it out. > > That test (written by me ;) ) essentially does > > src =3D malloc(pagesize); > dst =3D mmap(NULL, pagesize, PROT_READ, MAP_PRIVATE|MAP_ANON, -1, 0) > ... > > uffdio_copy.dst =3D (unsigned long) dst; > uffdio_copy.src =3D (unsigned long) src; > uffdio_copy.len =3D pagesize; > uffdio_copy.mode =3D 0; > if (ioctl(uffd, UFFDIO_COPY, &uffdio_copy)) { > ... > > > So src might not be aligned to a full page. > > According to the man page: > > "EINVAL Either dst or len was not a multiple of the system page size, or > the range specified by src and len or dst and len was invalid." > > So, AFAIKT, there is no requirement for src to be page-aligned. > > Using validate_range() on the src is wrong. Thanks for the report and the suggestions! I sent a fixup patch which should resolve this [1]. At least, I ran the test in question a bunch of times and it passed reliably with this fix. [1]: https://patchwork.kernel.org/project/linux-mm/patch/20230810192128.185= 5570-1-axelrasmussen@google.com/ > > -- > Cheers, > > David / dhildenb >