From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f172.google.com (mail-yw1-f172.google.com [209.85.128.172]) (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 4151339F160 for ; Fri, 10 Jul 2026 07:50:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669806; cv=none; b=XHMjGoQi/0ofW5C1FwXwrQ3KQddjyWomLFEWpLzKQE4taCsxGi/ynXc3LuuiMHXiE5rxdoEUBmZAZRnunOR0ITkQ1bWeWKQSSeGLkChk6JfKt9uJP0fKBceIyAIyuoRwaSQAxxuXZy/Fa2LUrp1GXGeG1D18b4VuSI5F0zLQAmo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669806; c=relaxed/simple; bh=WXJM06sR3l2mAwn1xmgBvQPu7vymNyKh5ym5WGLq0yI=; h=From:Date:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Nwo802uHwoKZ0wdOtiXjDZkTolMniVjzzfAQIxIZRLDekdp/KGlGgRi9qkqyZrw6spdbtsE2s64KgJUixSYossaqVVcjjaLWc4/hOXLg7BCYFxkOrl8VMibzcFY6mteiCt+dASI3yHfdhXdNk/B6jseqRiVaOoVjxXXtDsSAa1g= 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=UxaV5bTZ; arc=none smtp.client-ip=209.85.128.172 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="UxaV5bTZ" Received: by mail-yw1-f172.google.com with SMTP id 00721157ae682-7dbcb505578so6813647b3.3 for ; Fri, 10 Jul 2026 00:50:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783669804; x=1784274604; 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=wCAmxz9yPqRWofgVSMKF/vo2jh71M7LEVaHKWn5/sH8=; b=UxaV5bTZ0BRcyw0dq/K+wRrPrOqPIrNITWIwI6Ep8XwIjOgU93EEKcwXRN7oPF0LYV cnGC5Lwd3afQ3Dnr+XuBtSLpkf7bc9jwbejrdHSk90c0gsxPlPqVQO84wsntV27Ta4AO libmGhtppPvkYMRNTAhCdnCwPwaO7ZaPHcxp4T8cgutXiPWPgQHtCx1iq6xwxkkpjbcy k9IqY6XRhJyYGX18+h8lpKjwJ3DdWnWZcwa8QRlOg56iF+lgmmXFHCXsr0BM6vWwjV8J +VoW0sfuxSUuYCI0kHYoCLwjX/ent4qb11CByNudzwyqNa6mwczMK9e1RKwNHN1jpmAI 4vCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783669804; x=1784274604; 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=wCAmxz9yPqRWofgVSMKF/vo2jh71M7LEVaHKWn5/sH8=; b=iNICm6h3hgsTP/qD6BsK5CaWptncCeTn3/ZQhJi2JV+wgRCsiA5qizS60blkLixMHh vh4AZ55CzTQxoG9ET1+VXKY6kKsizluySoVYQpRPCfTsgzdyXmW3tL8GvcxfoV2RoxGr mVPwClo8qHJ9F8c/NJYjWOD3cySgGx5ITVKwuROwaXygS5EFoUwK80IruOZoeJeKV8m2 SnLHxEF+wHeh/2C4nC10QJF5WuORqrfCjn6GkJo9X8nq7PytEzUUwjyIgzrz32DGj3zQ n6HXXxMtUVsIeW+Op6IXgiESB1gDHGDOYZkGxLEXf8FrjJXRsTgQDCq+hYCdkdYpVRbk 2AzQ== X-Forwarded-Encrypted: i=1; AHgh+RrzDKJtYfzM4RFN5lKIhajJ3NRPHBepniVImMnYCXcFCLw5c4o+wkNBPZxT3qe7RmsTvCS5Mc8JOjSwPRQ=@vger.kernel.org X-Gm-Message-State: AOJu0YyKA+eHqmuemxk9GlGlHlLcubUGlSDP6npi42UTwtBThtoOv35e P6G3e8a5T6o9eSMyoq6mpzj+WJD4tCa0pqhEVFwmwBuZ3RbeutJPEXq6 X-Gm-Gg: AfdE7cmPYtMkUIzlPgU7/FD9CIng6BQkR/MrrJUvGXZWeEwDXBnHr0/BuquU0908c9Y InBOmk60gykfvVNetuspqd/vbjOiLG3tr+N6zQRbozI0iq2e6Y5lm7LH6dr9SoDU8CwrUIvpIEp jXgaVYELtk6iGXcvpGKyatqHuUZdyVjBOAgReTN/gzxLF95nzmxyHQQLAdGLwb54RWzXfoWfuLL ICttyopYZJEbo3UgTZh9smEC6EiFvFY48Jgn2jYbL8I7CS9feHUvVMzbl9Ow+04Ps7LO6EOXZEU xH4Fhk5EYscT3PvaRPvauhTie0hH1OdV3VoSN0m0bcMf8VKggRD6eRBWhpbmuJcrWKP/hM9EKsn 39nh82vufYK5+qGULU518qFWeWseupvXuYv5uNJwVefdtlnACn8MoY8DcuLAEg1oXNC4z2BIM34 v/D5fq0TFiOsL3gL74l4Q24Eivy+sStg8NV/rkWonMpw== X-Received: by 2002:a05:690c:6885:b0:81e:76f0:dbce with SMTP id 00721157ae682-81e76f0dde3mr28503047b3.26.1783669804054; Fri, 10 Jul 2026 00:50:04 -0700 (PDT) Received: from localhost (tpcc3a204.netvigator.com. [219.76.18.204]) by smtp.gmail.com with ESMTPSA id 00721157ae682-81e6c1feb43sm39038737b3.40.2026.07.10.00.50.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2026 00:50:03 -0700 (PDT) From: Coiby Xu X-Google-Original-From: Coiby Xu Date: Fri, 10 Jul 2026 15:47:09 +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 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall Message-ID: References: <20260501234342.2518281-1-coiby.xu@gmail.com> <20260501234342.2518281-4-coiby.xu@gmail.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=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Fri, May 08, 2026 at 09:08:39PM +0800, Coiby Xu wrote: >On Wed, May 06, 2026 at 07:26:16PM +0530, Sourabh Jain wrote: >> >> >>On 02/05/26 05:13, Coiby Xu wrote: >>>If writing to the configfs group happens concurrently during >>>kexec_file_load syscall, it may lead to the following issues, >>> - buffer overflow if dm-crypt keys are added after allocation >>> - stale total_keys if dm-crypt keys are removed during iteration >>> - keys_header will not be freed if config/crash_dm_crypt_key/reuse is >>> set true >>> >>>So hold config_keys_subsys.su_mutex for the entire sequence during the >>>kexec_file_load syscall to ensure a consistent snapshot. >> >> >>Yes, this will solve many synchronization problems when a  user tries to >>perform any operation under our configfs_subsystem while the kernel is >>executing the kexec_file_load system call. >> >>Now, regarding the third point about freeing key_header: this will work >>only if configfs takes the su_mutex lock before invoking the store callback. >>I am not sure whether it actually does. > >I can confirm configfs will automatically acquire the su_mutex lock when >creating configfs item. But I forgot to try if writing to an item will >also acquire the mutux lock. I'll do an experiment and share the result >later. You are right that writing to an attribute of an configfs item won't be protected by the mutux lock. Thanks for raising the concern! > >> >>However, based on my previous comment on (2/9), if we keep >>config_keys_reuse_store() >>under the kexec lock, this will be taken care of. This is because the entire >>kexec_file_load system call already runs under the kexec lock. >> >> >>> >>>Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory") >>>Signed-off-by: Coiby Xu >>>--- >>> kernel/crash_dump_dm_crypt.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c >>>index 4d8a3331bbe7..6377ee86ec50 100644 >>>--- a/kernel/crash_dump_dm_crypt.c >>>+++ b/kernel/crash_dump_dm_crypt.c >>>@@ -429,6 +429,7 @@ int crash_load_dm_crypt_keys(struct kimage *image) >>> }; >>> int r = 0; >>>+ mutex_lock(&config_keys_subsys.su_mutex); >>> if (key_count <= 0) { >>> kexec_dprintk("No dm-crypt keys\n"); >>>@@ -479,6 +480,9 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image) >>> kfree_sensitive(keys_header); >>> keys_header = NULL; >>> } >>>+ >>>+ if (mutex_is_locked(&config_keys_subsys.su_mutex)) >>>+ mutex_unlock(&config_keys_subsys.su_mutex); >>How about release the lock in crash_load_dm_crypt_keys() itself? >>Given that config_keys_reuse_store() is placed under kexec lock? > >Thanks for proposing another solution! I'll give a try and make a >comparison. After comparing the kexec lock approach with the configfs mutex lock approach, I think the latter is a simpler solution because 1. the kexec lock is non-blocking and we have to repeatedly try until the lock get acquired. So it means user space has to make changes as well. 2. configfs already acquires the mutex lock automatically for creating/deleting configfs items. So if we use configfs mutex lock, it means one less place to use the lock. So I prefer the configfs mutex lock approach unless I miss something, Sorry it took me long time to get back to you. I got a bit sidetracked by things including exploring ways to prevent these issues using unit tests, Smatch or even rust. But I think it's better I do them in a separate patch set. -- Best regards, Coiby