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 0B63CC4332F for ; Wed, 9 Nov 2022 11:26:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 760756B0072; Wed, 9 Nov 2022 06:26:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6E8AD6B0073; Wed, 9 Nov 2022 06:26:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5B0418E0001; Wed, 9 Nov 2022 06:26:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 4715F6B0072 for ; Wed, 9 Nov 2022 06:26:14 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 10CD11213BB for ; Wed, 9 Nov 2022 11:26:09 +0000 (UTC) X-FDA: 80113674858.29.A5E8A5B Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by imf08.hostedemail.com (Postfix) with ESMTP id 76022160009 for ; Wed, 9 Nov 2022 11:26:04 +0000 (UTC) Received: from neptune (ip5f592f1a.dynamic.kabel-deutschland.de [95.89.47.26]) by linux.microsoft.com (Postfix) with ESMTPSA id 1B3D1205DC1C; Wed, 9 Nov 2022 03:25:02 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1B3D1205DC1C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1667993107; bh=zUwbg/HEyF+bezTrnaFC6nz76szbZgSmnWrOoGNXm+s=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Hn8MKJT/RjE39bY8km6r3nNO43pooj3ElbJ48yMi2AD+e8wHBh51flFON7siZhHRp mLcXyuPZsf9P+60gg1ewlnJm29k4ElLfV44jMTYN5kss4lYlj/egI9D8+QMTA9NO23 QUtJqAXqxBPJJSvpATB8AyHbswG8r5BqdqtxtMAg= Date: Wed, 9 Nov 2022 12:23:38 +0100 From: Alban Crequy To: Yonghong Song Cc: Francis Laniel , linux-kernel@vger.kernel.org, Alban Crequy , Alban Crequy , Andrew Morton , Andrii Nakryiko , Mykola Lysenko , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Shuah Khan , linux-mm@kvack.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [RFC PATCH v1 1/2] maccess: fix writing offset in case of fault in strncpy_from_kernel_nofault() Message-ID: <20221109122338.7e931640@neptune> In-Reply-To: References: <20221108195211.214025-1-flaniel@linux.microsoft.com> <20221108195211.214025-2-flaniel@linux.microsoft.com> Organization: Microsoft X-Mailer: Claws Mail 4.1.1 (GTK 3.24.34; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667993164; a=rsa-sha256; cv=none; b=l/K5qUEpFsvOsjzZTX9EE/qIEbLBPJssuYPxnXmDeVyb2DA6xUde6PIz3iheUmhfeUfH96 mJzTF2aHl4f9hHtl8Im6bkf2iM9PB6UD//fTVbbVq0eQG/Kyar16kJEeZyFxeRqk9kV3qx Q+PI6gvw9GfLakVFGRylh8cikLH9Owc= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=linux.microsoft.com header.s=default header.b="Hn8MKJT/"; spf=pass (imf08.hostedemail.com: domain of albancrequy@linux.microsoft.com designates 13.77.154.182 as permitted sender) smtp.mailfrom=albancrequy@linux.microsoft.com; dmarc=pass (policy=none) header.from=linux.microsoft.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667993164; 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=zUwbg/HEyF+bezTrnaFC6nz76szbZgSmnWrOoGNXm+s=; b=KD/Sx68O1ku9xPNtTUj3cWTLx/sPaHrfzSfbLRYe+ouVtygPWvmiHGwDGbfdVNSnfh13tI Wn+ZrvGPz0r4+v3EahgWkbti2L7DlSHDjEQ6nU+79larY7ouNjBjAJre0xmbFxFS86N5fj BEB9cNUvWq0HjovMblL8AhMoaLzGdq8= Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=linux.microsoft.com header.s=default header.b="Hn8MKJT/"; spf=pass (imf08.hostedemail.com: domain of albancrequy@linux.microsoft.com designates 13.77.154.182 as permitted sender) smtp.mailfrom=albancrequy@linux.microsoft.com; dmarc=pass (policy=none) header.from=linux.microsoft.com X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 76022160009 X-Stat-Signature: anxswzc77hrj8xsgh9qncdc1zotorkx1 X-HE-Tag: 1667993164-219851 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, 8 Nov 2022 12:38:53 -0800 Yonghong Song wrote: > On 11/8/22 12:35 PM, Yonghong Song wrote: > >=20 > >=20 > > On 11/8/22 11:52 AM, Francis Laniel wrote: =20 > >> From: Alban Crequy > >> > >> If a page fault occurs while copying the first byte, this function=20 > >> resets one > >> byte before dst. > >> As a consequence, an address could be modified and leaded to > >> kernel crashes if > >> case the modified address was accessed later. > >> > >> Signed-off-by: Alban Crequy > >> Tested-by: Francis Laniel > >> --- > >> =C2=A0 mm/maccess.c | 2 +- > >> =C2=A0 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/mm/maccess.c b/mm/maccess.c > >> index 5f4d240f67ec..074f6b086671 100644 > >> --- a/mm/maccess.c > >> +++ b/mm/maccess.c > >> @@ -97,7 +97,7 @@ long strncpy_from_kernel_nofault(char *dst, > >> const void *unsafe_addr, long count) > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return src - unsafe_addr; > >> =C2=A0 Efault: > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pagefault_enable(); > >> -=C2=A0=C2=A0=C2=A0 dst[-1] =3D '\0'; > >> +=C2=A0=C2=A0=C2=A0 dst[0] =3D '\0'; =20 > >=20 > > What if the fault is due to dst, so the above won't work, right? > >=20 > > The original code should work fine if the first byte copy is > > successful. For the first byte copy fault, maybe just to leave it > > as is? =20 >=20 > Okay, the dst is always safe (from func signature), so change looks > okay to me. Probably mm people can double check. My understanding was that the bpf verifier is supposed to check that the dst pointer is safe. But I don't know where it is done, and I don't know how it can check that the dst buffer is big enough. > > =20 > >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return -EFAULT; > >> =C2=A0 } > >> > >> --=20 > >> 2.25.1 > >> =20 >=20