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 33C01C433FE for ; Tue, 18 Oct 2022 20:08:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AC7DA6B0078; Tue, 18 Oct 2022 16:08:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A781D6B007B; Tue, 18 Oct 2022 16:08:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 93F7A8E0001; Tue, 18 Oct 2022 16:08:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 829326B0078 for ; Tue, 18 Oct 2022 16:08:06 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3FCFDAB3E8 for ; Tue, 18 Oct 2022 20:08:06 +0000 (UTC) X-FDA: 80035156572.24.5B4162E Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by imf05.hostedemail.com (Postfix) with ESMTP id 1DF2E100030 for ; Tue, 18 Oct 2022 20:08:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666123685; x=1697659685; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=Wzg2WCQ/apCPx1qt4T8P5/FjRGp3MoH16v0Wjx66z4Y=; b=SUAT1gNSy67Zv3MUGJPvBMhfa4rJYEln7r4V09Ajn8dfHWjhgVU3l7we wnAThKvY6FJ95u5STNEpSLvCKp84UBVdBkIU5u8JF7DAFdqjpy+CJHKP1 FojI9z1bKKfOd3fqbz9QseK7X+ZhutSMf8uX1gLvjsgONQvngpefHlQm7 82+UIEQfwm6Ns/sBPcNYwc8eDjE5Z64n4apOK9Wo4Kj3spsckGsCpzqpn U/H1MAX2Nv3G2XiNkrAWomhVFpwT9ge6uRSNMq9IaFypt5J4nVCzdz5qJ e4etKHR0wc+lTMUW5pqLxLw9WXqkq7NahnjZumOlWC6+sR+H6fmf6u5+e Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10504"; a="368277555" X-IronPort-AV: E=Sophos;i="5.95,194,1661842800"; d="scan'208";a="368277555" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Oct 2022 13:08:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10504"; a="606724501" X-IronPort-AV: E=Sophos;i="5.95,194,1661842800"; d="scan'208";a="606724501" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga006.jf.intel.com with ESMTP; 18 Oct 2022 13:08:01 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1okssZ-009XLG-0r; Tue, 18 Oct 2022 23:07:59 +0300 Date: Tue, 18 Oct 2022 23:07:59 +0300 From: Andy Shevchenko To: Jane Chu Cc: Petr Mladek , "rostedt@goodmis.org" , "senozhatsky@chromium.org" , "linux@rasmusvillemoes.dk" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" Subject: Re: [PATCH] vsprintf: protect kernel from panic due to non-canonical pointer dereference Message-ID: References: <20221017191611.2577466-1-jane.chu@oracle.com> <5d987403-a7bf-8996-d639-c99edeaabcdf@oracle.com> <799e5390-2ff5-02b7-2df7-61198d5451e2@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <799e5390-2ff5-02b7-2df7-61198d5451e2@oracle.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1666123686; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/FcRzSieSLZeDs2hZsaCI49pAMiyDCeoSYeuFOq50Kc=; b=hOAO435KpooZfo85eu2yXZENBEtDeL3gU3cAhpVqthLSpUF4egJQv9rP4nnjHsNZDx2NwK TUPVoFLZHXs1Z99Z/s+jnIfvvEOn0+LhANcJCIIAbHk+gqTGlDp9Z65tEv+8IYZwM3a8Uj Dhu9aVqa6Em9Tm3wnZeXSuNbAxnd64Q= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=SUAT1gNS; spf=none (imf05.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 134.134.136.31) smtp.mailfrom=andriy.shevchenko@linux.intel.com; dmarc=fail reason="No valid SPF" header.from=intel.com (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1666123686; a=rsa-sha256; cv=none; b=3dO7uqZtPgfideuLHHtnsSDRVwwmQWhkvjvXWGOLgVHXzfbIgReWnygUOLg+KM+/t6IcxC IEvsR25dlia3BEfeFHMDBZpxVX6TWIP9HvXu0VzS7pLLZYmnCOCenP+BevsedDGXDLuzL6 b6DgqFmM7R4rnoa6mU3EEeO5ciYwRpI= X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1DF2E100030 X-Rspam-User: Authentication-Results: imf05.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=SUAT1gNS; spf=none (imf05.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 134.134.136.31) smtp.mailfrom=andriy.shevchenko@linux.intel.com; dmarc=fail reason="No valid SPF" header.from=intel.com (policy=none) X-Stat-Signature: j7oc5b7n53tgmkq5du14c1bf9a8rajti X-HE-Tag: 1666123684-142909 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, Oct 18, 2022 at 06:56:31PM +0000, Jane Chu wrote: > On 10/18/2022 5:45 AM, Petr Mladek wrote: > > On Mon 2022-10-17 19:31:53, Jane Chu wrote: > >> On 10/17/2022 12:25 PM, Andy Shevchenko wrote: > >>> On Mon, Oct 17, 2022 at 01:16:11PM -0600, Jane Chu wrote: > >>>> While debugging a separate issue, it was found that an invalid string > >>>> pointer could very well contain a non-canical address, such as > >>>> 0x7665645f63616465. In that case, this line of defense isn't enough > >>>> to protect the kernel from crashing due to general protection fault > >>>> > >>>> if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr)) > >>>> return "(efault)"; > >>>> > >>>> So instead, use kern_addr_valid() to validate the string pointer. > >>> > >>> How did you check that value of the (invalid string) pointer? > >>> > >> > >> In the bug scenario, the invalid string pointer was an out-of-bound > >> string pointer. While the OOB referencing is fixed, > > > > Could you please provide more details about the fixed OOB? > > What exact vsprintf()/printk() call was broken and eventually > > how it was fixed, please? > > For sensitive reason, I'd like to avoid mentioning the specific name of > the sysfs attribute in the bug, instead, just call it "devX_attrY[]", > and describe the precise nature of the issue. > > devX_attrY[] is a string array, declared and filled at compile time, > like > const char const devX_attrY[] = { > [ATTRY_A] = "Dev X AttributeY A", > [ATTRY_B] = "Dev X AttributeY B", > ... > [ATTRY_G] = "Dev X AttributeY G", > } > such that, when user "cat /sys/devices/systems/.../attry_1", > "Dev X AttributeY B" will show up in the terminal. > That's it, no more reference to the pointer devX_attrY[ATTRY_B] after that. > > The bug was that the index to the array was wrongfully produced, > leading up to OOB, e.g. devX_attrY[11]. The fix was to fix the > calculation and that is not an upstream fix. > > > > >> the lingering issue > >> is that the kernel ought to be able to protect itself, as the pointer > >> contains a non-canonical address. > > > > Was the pointer used only by the vsprintf()? > > Or was it accessed also by another code, please? > > The OOB pointer was used only by vsprintf() for the "cat" sysfs case. > No other code uses the OOB pointer, verified both by code examination > and test. So, then the vsprintf() is _the_ point to crash and why should we hide that? Because of the crash you found the culprit, right? The efault will hide very important details. So to me it sounds like I like this change less and less... > Here is a snippet of the crash backtrace from an instrumented kernel, > scratched one line for sensitive reason - > > crash> bt > PID: 3250 TASK: ffff9cb50fe23d80 CPU: 18 COMMAND: "cat" > #0 [ffffc0bacf377998] machine_kexec at ffffffff9b06c7c1 > #1 [ffffc0bacf3779f8] __crash_kexec at ffffffff9b13bb52 > #2 [ffffc0bacf377ac8] crash_kexec at ffffffff9b13cdac > #3 [ffffc0bacf377ae8] oops_end at ffffffff9b03357a > #4 [ffffc0bacf377b10] die at ffffffff9b033c32 > #5 [ffffc0bacf377b40] do_general_protection at ffffffff9b030c52 > #6 [ffffc0bacf377b70] general_protection at ffffffff9ba03db4 > [exception RIP: string_nocheck+19] > RIP: ffffffff9b87cc73 RSP: ffffc0bacf377c20 RFLAGS: 00010286 > RAX: 0000000000000000 RBX: ffff9da13fc17fff RCX: ffff0a00ffffff04 > RDX: 726f635f63616465 RSI: ffff9da13fc17fff RDI: ffffffffffffffff > RBP: ffffc0bacf377c20 R8: ffff9da0bfd2f010 R9: ffff9da0bfc18000 > R10: 0000000000001000 R11: 0000000000000000 R12: 726f635f63616465 > R13: ffff0a00ffffff04 R14: ffffffff9c1a6a4f R15: ffffffff9c1a6a4f > ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 > #7 [ffffc0bacf377c28] string at ffffffff9b87ce98 > #8 [ffffc0bacf377c58] vsnprintf at ffffffff9b87efe3 > #9 [ffffc0bacf377cb8] sprintf at ffffffff9b87f506 > #10 [ffffc0bacf377d18] <------------------------------> > #11 [ffffc0bacf377d28] dev_attr_show at ffffffff9b56d183 > #12 [ffffc0bacf377d48] sysfs_kf_seq_show at ffffffff9b3272dc > #13 [ffffc0bacf377d68] kernfs_seq_show at ffffffff9b32576c > #14 [ffffc0bacf377d78] seq_read at ffffffff9b2be407 > #15 [ffffc0bacf377de8] kernfs_fop_read at ffffffff9b325ffe > #16 [ffffc0bacf377e28] __vfs_read at ffffffff9b2940ea > #17 [ffffc0bacf377eb0] vfs_read at ffffffff9b2942ac > #18 [ffffc0bacf377ee0] sys_read at ffffffff9b29485c > #19 [ffffc0bacf377f28] do_syscall_64 at ffffffff9b003ca9 > #20 [ffffc0bacf377f50] entry_SYSCALL_64_after_hwframe at ffffffff9ba001b1 > > crash> dis ffffffff9b87cc73 > 0xffffffff9b87cc73 : movzbl (%rdx),%r8d > > and RDX: 726f635f63616465 was a non-canonical address. > > After applying this patch to the instrumented kernel, instead of panic, > the "cat" command produced "(efault)" > > > > > I wonder if this patch would prevent the crash or if the broken > > kernel would crash later anyway. > > A broken kernel has a different issue to be fixed, the upstream kernel > isn't broken, it could just offer better protect in case a bug was > introduced in future. -- With Best Regards, Andy Shevchenko