From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f52.google.com (mail-yx1-f52.google.com [74.125.224.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE93E2F362B for ; Fri, 10 Jul 2026 07:52:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669950; cv=none; b=S6cD59KkL7e7XWQ301qFb4yVWBDIBaIG5Gm0cz2eVRb8zIpgXxilubYgtf9n6WmtDIWQiC5LAg2Gr38LC/8a3CLeUPJGTGyRQAWJCOImcmLoB07v7sSOMIGTKVjcPZ1vHsCtsP6hERyEAOaixYuskia2KaUs81Vc4AbHQ9GOvNs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669950; c=relaxed/simple; bh=Ei5C8XW8c/wg8BqFtSRPOR1e10HUTwkBh9uAIKs2rLg=; h=From:Date:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sbFAlvUpF7/60iObmczPyp6vNgteCWlba7g9PN2UOWNo+KFCAEV42ETQQNPqETzO84bOsWn7AAcuU7AoJJY73JNAicJV9FGtnygJiLUpS7CQb0J7OQNArsBo7qsMfVtelc0HYhKPDBbqgrpXquhXCOWtCrH6nModS0ppr5hIWTc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=qx185QZl; arc=none smtp.client-ip=74.125.224.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qx185QZl" Received: by mail-yx1-f52.google.com with SMTP id 956f58d0204a3-664d78637f8so952290d50.3 for ; Fri, 10 Jul 2026 00:52:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783669948; x=1784274748; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:date :from:from:to:cc:subject:date:message-id:reply-to:content-type; bh=6ShLnV8gFSlWWUTTTcWYbLbIKdnzp7WaWpPnsAa0/Uk=; b=qx185QZlLziiX4pemDmIfMiaPmYOoB6e4gHccWJUSgEMmsY6MGMSYEgQiKmczp/Kug ZASeLPgClNSbapjCFYTm2Q8ZkJPCCcUo3v8rKh4DFmxt46WwTETj/9991E66/QOgoE/E AmmeYPpl/bi91iqRh8lEPR+1WscmjkMPM8HLUFHhjAmK7zZtx5XA1mGJafQat4U80XzD fJbi8gWSSHBc5lVnRTg8oLgyz2j6DYItLbPG/2Vt+5ZuT5WhoWAsRaxw7JPcxc/QU7/F gEaDfI8RgthX0MnMv490B3TUMCUrrsVk8EXENVgp5gA2EV7DdaY1hiTlxKXUT+IF6onw YfRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783669948; x=1784274748; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:date :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=6ShLnV8gFSlWWUTTTcWYbLbIKdnzp7WaWpPnsAa0/Uk=; b=HT0XVBkeZR97MLapu2vyLSGQC3WtQwgEVnCvIyrf/w/RRXSk2r67m0v7aKaNTEpKsu SVJF9eSa1F6BgZKASQKluRbeKIe5Wq/VYA1CRXJECcaMJMY7w3JcnY4c0H+os5/0+IGr Jq8R406mnyioO2VicGzSz4mwjak0GqLPVOazZqLUarbwNQfvPf9OSrLgGEw6ZrLxMXrK pG8ADH6M8G1z6pF5VBq6KvEHCUTcMxotcrELhPfvgiBpVqKA7p3cj9tMfSRdrX50K+JY i7uBohVWe1R4VJJpKdAO68T3QBT1oZIdjPCanZYQ6o+oLetI1wFSNX0GNSHfhtk43v8j xjLg== X-Forwarded-Encrypted: i=1; AHgh+RqUZQqXSLzN5LDhLdbnRher8sCoZC8TXeNdKVXErVImBYrQ9JaVKAUvanlWvhNHI1dDVEQveKy7AsCKYMg=@vger.kernel.org X-Gm-Message-State: AOJu0YwApoRSjuygQR0S/fKrTGgwU30OOJWfROhAWbQMGI0qaqca5Vn6 8lvDjSesnNTVyc0BgzWyGTSQh0fBB6kr0ES8FJGaK+oOUkT5JA9Zcpkr X-Gm-Gg: AfdE7cnlm4DPsLztZSpKGwq7LVT1rbS9U9YiAAsXtYiYy1kBpxvD9M4UD6NCS01PEu9 loaSZIRsi2J5lVlDlIVzNtyDJOuo3mbGhyPC1QC6NuRkEJ2kBJQFuOzJV4jQgQ1wwq999hBA+Y6 wnr1LVVXcxqOMrBPBNBlrqKJ5coBlW6iCxWiA1gzfrTE8EUp6abVjw7rbDkzEmBGeBkn+9HJ10s WYv45f3CBJ2TWyhG70Ph/0kp9qe0IR374LvL3Qaa7gz+hJE55+qSikcIBE0Rc0hRHKymwKB1udq 9+34vFqZWCJHJ2LuRTodby3T3Bn9XgSiKNQT7oPT9reJfFKOz15MOITji7KEJ9duqYNmShbjzU1 2IGAMQDLeBfxxbYQr4yEayuEe+a04o7I6gax35YwFLrzJKcCyxGt4ouvT3GafnpT3D2+pueB49g QmAwHyFR570aXneSiWBXckmlm37JuhmP4= X-Received: by 2002:a05:690e:e8d:b0:667:b04d:c0ca with SMTP id 956f58d0204a3-667b04dd090mr6451470d50.69.1783669947817; Fri, 10 Jul 2026 00:52:27 -0700 (PDT) Received: from localhost (tpcc3a204.netvigator.com. [219.76.18.204]) by smtp.gmail.com with ESMTPSA id 956f58d0204a3-66787a5b4adsm6234059d50.18.2026.07.10.00.52.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2026 00:52:27 -0700 (PDT) From: Coiby Xu X-Google-Original-From: Coiby Xu Date: Thu, 14 May 2026 17:09:30 +0800 To: Sourabh Jain Cc: kexec@lists.infradead.org, Andrew Morton , Baoquan He , Dave Young , Mike Rapoport , Pasha Tatashin , Pratyush Yadav , Coiby Xu , open list Subject: Re: [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header Message-ID: References: <20260501234342.2518281-1-coiby.xu@gmail.com> <20260501234342.2518281-3-coiby.xu@gmail.com> <54a27f47-a7cd-458c-9b27-7b84949aa453@linux.ibm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <54a27f47-a7cd-458c-9b27-7b84949aa453@linux.ibm.com> On Tue, May 12, 2026 at 11:12:16AM +0530, Sourabh Jain wrote: > > >On 10/05/26 05:44, Coiby Xu wrote: >>On Sat, May 09, 2026 at 01:36:59AM +0530, Sourabh Jain wrote: >>> >>> >>>On 08/05/26 18:03, Coiby Xu wrote: >>>>On Wed, May 06, 2026 at 05:58:31PM +0530, Sourabh Jain wrote: >>>>>Hello Coiby, >>>> >>>>Hi Sourabh, >>>> >>>>Thanks for reviewing the patch! >>>> >>>>> >>>>>On 02/05/26 05:13, Coiby Xu wrote: >>>>>>If kexec_add_buffer somehow fails, keys_header will be >>>>>>freed. Depending >>>>>>on /sys/kernel/config/crash_dm_crypt_key/reuse, it will lead to the >>>>>>following two problems if the kexec_file_load syscall is >>>>>>called again, >>>>>>  1. Double free of keys_header if reuse=false >>>>>>  2. UAF of keys_header if reuse=true >>>>>> >>>>>>To address these problems and also make it easier to reason about the >>>>>>code, keep two invariants, >>>>>>  1. keys_header will always be freed at the end of kexec_file_load >>>>>>     syscall except during kdump image unloading for CPU/memory >>>>>>     hot-plugging support >>>>>>  2. There will always be valid keys_header if reuse=true >>>>>> >>>>>>Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in >>>>>>kdump reserved memory") >>>>>>Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys >>>>>>for CPU/memory hot-plugging") >>>>>>Reported-by: Sourabh Jain >>[...] >>>> >>>>> >>>>> >>>>> >>>>>>         kexec_dprintk( >>>>>>             "dm-crypt keys haven't be saved to >>>>>>crash-reserved memory\n"); >>>>>>         return -EINVAL; >>>>>>     } >>>>>>-    if (kstrtobool(page, &is_dm_key_reused)) >>>>>>+    if (kstrtobool(page, &val) || !val) >>>>>>         return -EINVAL; >>>>> >>>>>Why can’t we allow the user to set is_dm_key_reused = false >>>>>and free the >>>>>key_header? That way, the next kdump kernel load will recreate the >>>>>For example, a user may add more keys and want the new keys to >>>>>be included >>>>>in the kdump image from next kdump kernel load. >>>> >>>>Personally, I want to limit the reuse configfs API to the case of >>>>CPU/memory hotplug thus to make the code simpler. And for the case of a >>>>user adding more keys, I don't think there is a need for using >>>>the reuse >>>>API. >>> >>> >>>Yes, there is no need for it, but I think the user is forced to use >>>key_reuse because once it is set to true, it cannot be set back to >>>false. >>> >>>For example, the kdump kernel is loaded using the kexec_load system call >>>and reuse is set to true. The user may later add a couple of new >>>keys and >>>want to include them in the kdump image. >>> >>>I think the next kdump kernel load will reuse the key_headers even >>>though >>>the user does not want it. Hence I see a problem here. >>> >>>Well user can load kdump kernel a second time, and at that point the >>>keys will get updated. But do we really want this behavior? >> >>key_reuse won't be set to true automatically unless an user explicitly >>does so. Thus I don't think it's forcing users to use it. And by design >>key_reuse is set only for the hotplug case. > >Oh, I got it now. So, on kdump kernel load, key reuse will always be >set to false. > >And when the user wants to reuse the key, they can just set the key reuse to >true and reload the kdump kernel. After the reload, key reuse will be set to >false again by the kernel itself. > >Thanks for clearing up my doubts. My pleasure! Good to know we are on the same page now:) > [...] >>>>>>+    if (!is_dm_key_reused) { >>>>>>+        kfree_sensitive(keys_header); >>>>>>+        keys_header = NULL; >>>>> >>>>>Since crash_load_dm_crypt_keys() sets is_dm_key_reused = >>>>>false, keys_header will >>>>>always be released here, right? Then why is the above free >>>>>under an if condition? >>>> >>>>Thanks for raising the question! This is to prevent "kexec -u" from >>>>cleaning up keys_headers because kexec_file_post_load_cleanup_dm_crypt >>>>will also be called during "kexec -u". Without the if condition, >>>>keys_headers will not be available during reloading. >>> >>>Agree. But do we really run kexec -u and then kexec -p to reload >>>the kdump >>>kernel on hotplug events? I am under the impression that the udev rule >>>simply reloads the kdump kernel using kexec -p without explicitly >>>running >>>kexec -u. >> >>Yes, "kdumpctl reload" will be called during hotplug events https://github.com/rhkdump/kdump-utils/blob/main/kdump-udev-throttler#L51 >>So there is explicitly unloading first and then reloading. > >Oh ok, then it makes sense. > >BTW, I got the impression about not using kexec -u on kdump service reload >from the repo below: > >https://github.com/openSUSE/kdump/blob/master/70-kdump.rules.in [Udev Rule] >https://github.com/openSUSE/kdump/blob/00979dc621ba35285b85bf55917a5b0dfd273114/init/load-once.sh#L22 >[Wrapper to load once for multiple hotplug] >https://github.com/openSUSE/kdump/blob/00979dc621ba35285b85bf55917a5b0dfd273114/init/load.sh#L312 >[load kdump script] Thanks for sharing how openSUSE support hotplug! I believe current API shall also work for openSUSE if they plan to support LUKS-encrypted dump target. -- Best regards, Coiby