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 DE5B6C54E67 for ; Wed, 20 Mar 2024 12:07:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4693D6B0083; Wed, 20 Mar 2024 08:07:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 418E96B0085; Wed, 20 Mar 2024 08:07:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 293046B0088; Wed, 20 Mar 2024 08:07:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 128B46B0083 for ; Wed, 20 Mar 2024 08:07:20 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id E0CD941262 for ; Wed, 20 Mar 2024 12:07:19 +0000 (UTC) X-FDA: 81917292198.14.C880BD3 Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by imf10.hostedemail.com (Postfix) with ESMTP id 27CD7C0031 for ; Wed, 20 Mar 2024 12:07:16 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=sJmpy2XF; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of glider@google.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=glider@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710936437; 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=z7DDAmZLzRu6HgzyxGlka/nEAH/Bp9s7j8g062J1qR0=; b=t4zXeXGKCT4EFD8irR/A32bHClvH9URiWbgcmj+5S/KZug0xU/PybKh6X9A4poGrrMabFX 2qOd1jvRJTCaBSY2yWfxjxl/7Jg2alIkNFTWjcC3DeuEuk6oZhK9QhbKqh6VkR0biQH72Z nqz+eYNgoVTmXR44kXpZ1rUmcQ2pRxM= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=sJmpy2XF; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of glider@google.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=glider@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710936437; a=rsa-sha256; cv=none; b=dCht2Am8oJL4eqXN3uWVSY1ysOY6IYDsonsfc30cLZar65ZjVj61qxwOpswT3YrMk6EErM XCzcvpLq/u5k+46WQ4yVhUIVARqrReacP/jGMl/3sayHz+7Wi/RzVSV8CXy1JieBey4sUZ b/RB2sRYG4IZh0bTUV9DeOwjOAZ0wPQ= Received: by mail-qv1-f54.google.com with SMTP id 6a1803df08f44-690cf6ecd3cso38134146d6.2 for ; Wed, 20 Mar 2024 05:07:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1710936436; x=1711541236; darn=kvack.org; 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=z7DDAmZLzRu6HgzyxGlka/nEAH/Bp9s7j8g062J1qR0=; b=sJmpy2XFFiq76XjILk6DcgTkSUbRHt+DCO9ADewNvh9PXBf34ZRpGnJo3CaFIjfK0w 5jrOtam55QJOLo2aJQ2m6jVI57U4zlaNXt8aL6bGlGK4o6PnlZ1hd8j6iAWC0yNsMMKY nO+Tc0XymPR/a3QYcU0TBz6W5BwBbqfUZPvK2xao5U+xi5hhi1BIxvpOOapoRawHC/be PY0HUJrBgxOnoWDtEMb8vojUqglPtJgEGV1nZiPTKmuiLbbUp37XSZbH7V13P8Scj/xR go8yCwAzAtOxYrg6cJkJLJBGSYoXxGqtA61Gz5DJdFnopPavL0UK0n0cvOW8dL1K5yqF 0O4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710936436; x=1711541236; 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=z7DDAmZLzRu6HgzyxGlka/nEAH/Bp9s7j8g062J1qR0=; b=pxrrg+btEZiYZTRxGVf+3HQezfJgyehxNAUwd8w6PwXn7594738CYAa73TxIphLqFc muP7KCSuEMiC3YnYnQmiFzNW9czYpDiCzBAdqEStnU4rM9Zm0eN0c0Ti1rDtoXCW5XyX cx/h46kZ+g8Q6TkmksFMlwdyEZ/6NilnUd5xehdq1pp5N6/3WsXqU90ij/zRpStxCUmJ MKgB/pp/8X35CXdMiCCyB/JzMKs5V67Qsp4SrrC5WX21txoJ1J60ycZrSwal2FzZ6k5d tbRAhqj0WARQHp1YE44nYTVu7KjGUtFqHcmRPS42tXs1ApXfKBRtLJNMydmh/+UqfnCB AyzQ== X-Forwarded-Encrypted: i=1; AJvYcCXcvr6U0vacylXG7OuGw4lbV7SO6FaHQj05TDTG1PmO9JVmTa4zEZinPcoCaC0grybkosantr77qMKBZzlvy8XimtQ= X-Gm-Message-State: AOJu0YxlyIYs14GMtXI9cNYTVKj42wVx4mPdd7lOiqI89aIiZuwcf8jE jUu58sncYnS2b8rc8Kfik2yp36OyNa4koHRCJL7RU4QJE0r4qQ/UboV9p3bBGQ5apEBWf7ZX8Iw 1eFi/uqO88c1vQT6gRlJhYaoVf2mUozjSfXgN X-Google-Smtp-Source: AGHT+IFsZ1Kl+nOzqPW0+FjUKUPnqIsgeV3Xy3EhYghtu8MFkZrVbUNy3Po7/qq/bRXKj3pCCJ8hWquQq+UKPzKOHh8= X-Received: by 2002:a05:6214:2b97:b0:691:64e9:9a4a with SMTP id kr23-20020a0562142b9700b0069164e99a4amr25165795qvb.53.1710936436027; Wed, 20 Mar 2024 05:07:16 -0700 (PDT) MIME-Version: 1.0 References: <20240319163656.2100766-1-glider@google.com> <20240319163656.2100766-3-glider@google.com> In-Reply-To: From: Alexander Potapenko Date: Wed, 20 Mar 2024 13:06:33 +0100 Message-ID: Subject: Re: [PATCH v1 3/3] x86: call instrumentation hooks from copy_mc.c To: Tetsuo Handa Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kasan-dev@googlegroups.com, tglx@linutronix.de, x86@kernel.org, Linus Torvalds , Dmitry Vyukov , Marco Elver Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 27CD7C0031 X-Stat-Signature: tzzgyhc15mq9z67su7b7uktfu9mt4jb1 X-HE-Tag: 1710936436-129953 X-HE-Meta: U2FsdGVkX19tP7zgLc/enh3wIRRaN9vzKdMQ952zUY3zKce+tjUUyPd7ZXuOVFl3q+s21w5TA/z7MJdVaeUVMygNg69/jDoYuw/25Mc3zqTJpDzfzH7KqpN5wYfhsz3bj0mAQcbZSNEcl+Z2ZObC9UoE+8yju4qWyJKHsS0Ee4YNCJDo6Ge/o5i/GeqfctEDZZrezLj6cJVyiU+uwWMab0ocHTXvgWL8oHJFBebcKepVv/D+Em1V5K5bq2aM4+JOi+/8hElaEnd30InYlmXxZcbpfnEF1w91u1wP0AJYVRGBKmHWwofectDElRPdxkSG6ZQ8Q7tMp+64cLGthZLnTDSQ8AlQHr2IZTD89S0qNJ1xezNnjK20jL2Q2cwFIQBDq/FIPAYLqNTDgLeqC+2XswOulhtrAnuxSpbuGsE1aFjIokp4jqxJtwlEjHaEDY2XyuOSBCC/aCkj85aFQ7lj0NSs+lZOmmnTKlI8L4zirDxi9tKbddNRyYosoFQ7/kvJARtSgRFeWCcneY8aoLC2wnYfUVaCMOVNoPkFXOlzmux1ZQ5EFmLMehnKcsB42XcRBdwu0F3iuNxgB2CHAfGI+rzudR2qdjrVz9WuMy4Ua3eDiqnrbYUeO0dM67SWTOdTbX5q3dMJM7o80pati3FK/QiHXYfQ60k4tveu/C+kOxDfAq7/SQayJ7esLSMWdBdz1rl1K1E/1zQa7wyBDgMxml9vZ0CoEhKTji3HDyIqIzVvkkUpcZFVmrmOepfS7a2o/fijkFibw8TtLVUW6EptoASeyS8x1PL1yxkAkdLngKj7l3w3H4jBi5rm8CJCthvblJfEKc93ecartUtHvZYqfjykrdO1UA1CYm8wXLPboTR20SirEYSnSFPvlKUgiWR3bO3gAdsWZdYZ2I4mUNepahLKcRbItkgGtb7aCUYnjmUQUKgDQIWkEtdXqecbfMZHFkEKUxVIvFnEvx2FFUl 9g0ZSkgE v4OejmaYCGjV6nY15j9KOClj+HTChjH9GOwQi2WojVo4OFEqewOhP6h9PwBnJLH1B8iYHxQbzi6SILoLoOKLjvkLW4tS98fZDAxeMpKZQj92Lcy8cFWf6yvsUJuEHdfoabYso9idKUvONvcFAGri0pLlxaKUpWPbSKpEXK1w8cU6lMgwoBGKKW06Gjbt0/BfbFU/zOZffBMNPxhBrfxCQoH1kbGW7zNjHjJ542hDpYIrmwgA75MWalUTW96iRkTySKbu1t6o8zlY45KWLEo59Onq4C6D113Jm03N4G4MUCjTQ+pQ1wZhlI7e5TBsjO2zQ3DepA2AoN1gKk/XJhiD+l3l/JB7Sqg6kN3gyymFjjRa3KjPzeAO8+fwgbtZC+Q/Teb3+7qBd9CWyMlc= 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: List-Subscribe: List-Unsubscribe: On Wed, Mar 20, 2024 at 11:40=E2=80=AFAM Tetsuo Handa wrote: > > On 2024/03/20 18:29, Alexander Potapenko wrote: > > But for KASAN/KCSAN we can afford more aggressive checks. > > First, if we postpone them after the actual memory accesses happen, > > the kernel may panic on the invalid access without a decent error > > report. > > Second, even if in a particular case only `len-ret` bytes were copied, > > the caller probably expected both `src` and `dst` to have `len` > > addressable bytes. > > Checking for the whole length in this case is more likely to detect a > > real error than produce a false positive. > > KASAN/KCSAN care about whether the requested address range is accessible = but > do not care about whether the requested address range was actually access= ed? I am not exactly sure under which circumstances a copy_mc may fail, but let's consider how copy_to_user() is handled. In instrument_copy_to_user() (https://elixir.bootlin.com/linux/latest/source/include/linux/instrumented.= h#L110) we check the whole ranges before the copy is performed. Assume there is buggy code in the kernel that allocates N bytes for some buffer and then copies N+1 bytes from that buffer to the userspace. If we are (un)lucky enough, the userspace code may be always allocating the destination buffer in a way that prevents copy_to_user() from copying more than N bytes. Yet it is possible to provide a userspace buffer that is big enough to trigger an OOB read in the kernel, and reporting this issue is usually the right thing to do, even if it does not occur during testing. Moreover, if dst can receive N+1 bytes, but the OOB read happens to crash the kernel, we'll get a simple panic report instead of a KASAN report, if we decide to perform the check after copying the data. > > By the way, we have the same problem for copy_page() and I was thinking a= bout > https://lkml.kernel.org/r/1a817eb5-7cd8-44d6-b409-b3bc3f377cb9@I-love.SAK= URA.ne.jp . > But given that instrument_memcpy_{before,after} are added, > how do we want to use instrument_memcpy_{before,after} for copy_page() ? > Should we rename assembly version of copy_page() so that we don't need to= use > tricky wrapping like below? I think renaming the assembly version and providing a `static inline void copy_page()` in arch/x86/include/asm/page_64.h should be cleaner, but it is up to x86 people to decide. The patch below seems to work: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.= h index cc6b8e087192e..70ee3da32397e 100644 --- a/arch/x86/include/asm/page_64.h +++ b/arch/x86/include/asm/page_64.h @@ -8,6 +8,7 @@ #include #include +#include #include /* duplicated to the one in bootmem.h */ @@ -58,7 +59,14 @@ static inline void clear_page(void *page) : "cc", "memory", "rax", "rcx"); } -void copy_page(void *to, void *from); +void copy_page_asm(void *to, void *from); + +static inline void copy_page(void *to, void *from) +{ + instrument_memcpy_before(to, from, PAGE_SIZE); + copy_page_asm(to, from); + instrument_memcpy_after(to, from, PAGE_SIZE, 0); +} #ifdef CONFIG_X86_5LEVEL /* diff --git a/arch/x86/lib/copy_page_64.S b/arch/x86/lib/copy_page_64.S index d6ae793d08faf..e65b70406d48a 100644 --- a/arch/x86/lib/copy_page_64.S +++ b/arch/x86/lib/copy_page_64.S @@ -13,13 +13,13 @@ * prefetch distance based on SMP/UP. */ ALIGN -SYM_FUNC_START(copy_page) +SYM_FUNC_START(copy_page_asm) ALTERNATIVE "jmp copy_page_regs", "", X86_FEATURE_REP_GOOD movl $4096/8, %ecx rep movsq RET -SYM_FUNC_END(copy_page) -EXPORT_SYMBOL(copy_page) +SYM_FUNC_END(copy_page_asm) +EXPORT_SYMBOL(copy_page_asm) SYM_FUNC_START_LOCAL(copy_page_regs) subq $2*8, %rsp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D