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=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 2C0A4C43331 for ; Mon, 30 Mar 2020 17:17:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DD27520578 for ; Mon, 30 Mar 2020 17:17:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DD27520578 Authentication-Results: mail.kernel.org; dmarc=none (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 917EA6B006C; Mon, 30 Mar 2020 13:17:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8EFDE6B006E; Mon, 30 Mar 2020 13:17:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8054E6B0070; Mon, 30 Mar 2020 13:17:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0108.hostedemail.com [216.40.44.108]) by kanga.kvack.org (Postfix) with ESMTP id 6A8586B006C for ; Mon, 30 Mar 2020 13:17:55 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 1DFCD180AD807 for ; Mon, 30 Mar 2020 17:17:55 +0000 (UTC) X-FDA: 76652686110.07.neck05_622cf1f242f09 X-HE-Tag: neck05_622cf1f242f09 X-Filterd-Recvd-Size: 7093 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Mon, 30 Mar 2020 17:17:54 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BDAC9101E; Mon, 30 Mar 2020 10:17:53 -0700 (PDT) Received: from [172.16.1.108] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 541CA3F68F; Mon, 30 Mar 2020 10:17:52 -0700 (PDT) Subject: Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image To: David Hildenbrand Cc: kexec@lists.infradead.org, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, Eric Biederman , Andrew Morton , Catalin Marinas , Will Deacon , Anshuman Khandual , Bhupesh Sharma References: <20200326180730.4754-1-james.morse@arm.com> <20200326180730.4754-2-james.morse@arm.com> <321e6bf7-e898-7701-dd60-6c25237ff9cd@redhat.com> <9cb4ea0d-34c3-de42-4b3f-ee25a59c4835@redhat.com> <72672e2c-a57a-8df9-0cff-8035cbce7740@redhat.com> <34274b02-60ba-eb78-eacd-6dc1146ed3cd@arm.com> <80e4d1d7-f493-3f66-f700-86f18002d692@redhat.com> From: James Morse Openpgp: preference=signencrypt Message-ID: Date: Mon, 30 Mar 2020 18:17:48 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <80e4d1d7-f493-3f66-f700-86f18002d692@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Hi David, On 3/30/20 2:13 PM, David Hildenbrand wrote: >> Adding a sentence about the way kexec load works may help, the first paragraph >> would read: >> >> | Kexec allows user-space to specify the address that the kexec image should be >> | loaded to. Because this memory may be in use, an image loaded for kexec is not >> | stored in place, instead its segments are scattered through memory, and are >> | re-assembled when needed. In the meantime, the target memory may have been >> | removed. >> >> Do you think thats clearer? > > Yes, very much. Maybe add, that the target is described by user space > during kexec_load() and that user space - right now - parses /proc/iomem > to find applicable system memory. (I don't think x86 parses /proc/iomem anymore). I'll repost this patch with that expanded commit message, once we've agreed this is the right thing to do! >>> I wonder if we should instead make the "kexec -e" fail. It tries to >>> touch random system memory. >> >> Heh, isn't touching random system memory what kexec does?! > > Having a racy user interface that can trigger kernel crashes feels very > wrong. We should limit the impact. >> Its all described to user-space as 'System RAM'. Teaching it to probe >> /sys/devices/memory/... would require a user-space change. > > I think we should really rename hotplugged memory on all architectures. > > Especially also relevant for virtio-mem/hyper-v balloon, where some > pieces of (hotplugged )memory blocks are partially unavailable and > should not be touched - accessing them results in unpredictable behavior > (e.g., crashes or discarded writes). I'll need to look into these. I'd assume for KVM that virtio-mem can be brought back when its accessed ... its just going to be slow. >>> Will probably need some thought. But it will actually also bail out when >>> user space passes wrong physical memory addresses, instead of >>> triple-faulting silently. >> >> With this change, the reboot(LINUX_REBOOT_CMD_KEXEC), call would fail. This >> thing doesn't usually return, so we're likely to trigger error-handling that has >> never run before. >> >> (Last time I debugged one of these, it turned out kexec had taken the network >> interfaces down, meaning the nfsroot was no longer accessible) >> >> How can user-space know whether kexec is going to succeed, or fail like this? >> Any loaded kexec kernel could secretly be in this broken state. >> >> Can user-space know what caused this to become unreliable? (without reading the >> kernel source) >> >> >> Given kexec can be unloaded by user-space, I think its better to prevent us >> getting into the broken state, preferably giving the hint that kexec us using >> that memory. The user can 'kexec -u', then retry removing the memory. >> >> I think forbidding the memory-offline is simpler for user-space to deal with. > > I thought about this over the weekend, and I don't think it's the right > approach. > 1. It's racy. If memory is getting offlined/unplugged just while user > space is about to trigger the kexec_load(), you end up with the very > same triple-fault. load? How is this different to user-space providing a bogus address? Sure, user-space may take a nap between parsing /proc/iomem and calling kexec_load(), but the kernel should reject these as they would never work. (I can't see where sanity_check_segment_list() considers the platform's memory. If it doesn't, we should fix it) Once the image is loaded, and clashes with a request to remove the memory there are two choices: secretly unload the image, or prevent the memory being taken offline. > 2. It's semantically wrong. kexec does not need online memory ("managed > by the buddy"), but still you disallow offlining memory. It does need the memory if you want 'kexec -e' to succeed. If there were any sanity tests, they should have happened at load time. The memory is effectively in use by the loaded kexec image. User-space told the kernel to use this memory, you should not be able to then remove it, without unloading the kexec image first. Are you saying feeding bogus addresses to kexec_load() is _expected_ to blow up like this? > I would really much rather want to see user-space choosing boot memory > (e.g., renaming hotplugged memory on all architectures), and checking > during "kexec -e" if the selected memory is actually "there", before > trying to write to it. How does 'kexec -e' know where the kexec kernel was loaded? You'd need to pass something between 'load' and 'exec'. How do you keep existing user-space working as much as possible? What do you do if the memory isn't there? User-space just called reboot(), it would be better to avoid getting into the situation where we have to fail that call. Solving the bigger problem, would add a 'kexec_it_now' flag to the kexec_load() call. This would make the window where 'stuff' can change much smaller. Things changing while user-space sleeps isn't a solvable problem, these would need to be rejected by sanity tests at load time. Thanks, James