Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 5 Jun 2023 10:31:49 +0800
From:   Coiby Xu <coxu@redhat.com>
To:     Eric Biggers <ebiggers@kernel.org>,
        Milan Broz <gmazyland@gmail.com>
Cc:     kexec@lists.infradead.org, Baoquan He <bhe@redhat.com>,
        x86@kernel.org, dm-devel@redhat.com,
        Pingfan Liu <kernelfans@gmail.com>,
        linux-kernel@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
        Kairui Song <ryncsn@gmail.com>,
        Jan Pazdziora <jpazdziora@redhat.com>,
        Thomas Staudt <tstaudt@de.ibm.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Dave Young <dyoung@redhat.com>
Subject: Re: [PATCH 0/5] Support kdump with LUKS encryption by reusing LUKS
 volume key
Message-ID: <36mz3gn764ceadfbuhhmoo2zaiqmzplpkdcnszha2hzhmb3i62@sm6hilxryzk4>
References: <20230601072444.2033855-1-coxu@redhat.com>
 <20230602213452.GC628@quark.localdomain>
 <f569a35a-bff9-9dbb-2ded-7e02ee060a9a@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <f569a35a-bff9-9dbb-2ded-7e02ee060a9a@gmail.com>
Precedence: bulk

Hi Eric and Milan,

On Sat, Jun 03, 2023 at 11:22:52AM +0200, Milan Broz wrote:
>On 6/2/23 23:34, Eric Biggers wrote:
>>On Thu, Jun 01, 2023 at 03:24:39PM +0800, Coiby Xu wrote:
>>>[PATCH 0/5] Support kdump with LUKS encryption by reusing LUKS volume key
>>
>>The kernel has no concept of LUKS at all.  It provides dm-crypt, which LUKS
>>happens to use.  But LUKS is a userspace concept.
>>
>>This is a kernel patchset, so why does it make sense for it to be talking about
>>LUKS at all?  Perhaps you mean dm-crypt?
>
>Exactly.

Thanks for raising the above concern! The use cases like CoreOS and
Confidential VMs explicitly want kdump to work for LUKS. And correct me
if I'm wrong, I think the two problems addressed by this patch set only
apply to LUKS so the kdump part of the kernel only cares about the LUKS
case. If there are use cases where similar approach is needed, I'll be
happy to make the solution more generic.

>
>I had the same comment almost a year ago... and it still applies:
>https://lore.kernel.org/all/c857dcf8-024e-ab8a-fd26-295ce2e0ae41@gmail.com/
>
> Anyway, please fix the naming before this patchset can be read or reviewed!
>
> LUKS is user-space key management only (on-disk metadata); the kernel has
> no idea how the key is derived or what LUKS is - dm-crypt only knows the key
> (either through keyring or directly in the mapping table).
>
> Polluting kernel namespace with "luks" names variables is wrong - dm-crypt
> is used in many other mappings (plain, bitlocker, veracrypt, ...)
> Just use the dm-crypt key, do not reference LUKS at all.

Thanks for the reminding! That comment was on the first RFC version. But
starting with "RFC v2", there is no longer any interaction with dm-crypt
(to save a copy of the LUKS volume key for the kdump kernel) and now I
make cryptsetup talks to the kdump part of the kernel via the sysfs to
reuse the volume key. So only the kdump part of the kernel needs to know
LUKS which is what it cares. Thus I don't think there is any kernel
namespace pollution now.

>
>Milan
>

-- 
Best regards,
Coiby