linux-kernel - Re: [PATCH v1 01/11] fs/proc/vmcore: convert vmcore_cb_lock into vmcore

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZzcUpoDJ2xPc3FzF@MiWiFi-R3L-srv>
Date: Fri, 15 Nov 2024 17:30:14 +0800
From: Baoquan He <bhe@...hat.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	linux-s390@...r.kernel.org, virtualization@...ts.linux.dev,
	kvm@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	kexec@...ts.infradead.org, Heiko Carstens <hca@...ux.ibm.com>,
	Vasily Gorbik <gor@...ux.ibm.com>,
	Alexander Gordeev <agordeev@...ux.ibm.com>,
	Christian Borntraeger <borntraeger@...ux.ibm.com>,
	Sven Schnelle <svens@...ux.ibm.com>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	Vivek Goyal <vgoyal@...hat.com>, Dave Young <dyoung@...hat.com>,
	Thomas Huth <thuth@...hat.com>, Cornelia Huck <cohuck@...hat.com>,
	Janosch Frank <frankja@...ux.ibm.com>,
	Claudio Imbrenda <imbrenda@...ux.ibm.com>,
	Eric Farman <farman@...ux.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v1 01/11] fs/proc/vmcore: convert vmcore_cb_lock into
 vmcore_mutex

On 10/25/24 at 05:11pm, David Hildenbrand wrote:
> We want to protect vmcore modifications from concurrent opening of
> the vmcore, and also serialize vmcore modiciations. Let's convert the


> spinlock into a mutex, because some of the operations we'll be
> protecting might sleep (e.g., memory allocations) and might take a bit
> longer.

Could you elaborate this a little further. E.g the concurrent opening of
vmcore is spot before this patchset or have been seen, and in which place
the memory allocation is spot. Asking this becasue I'd like to learn and
make clear if this is a existing issue and need be back ported into our
old RHEL distros. Thanks in advance.


> 
> Signed-off-by: David Hildenbrand <david@...hat.com>
> ---
>  fs/proc/vmcore.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index b52d85f8ad59..110ce193d20f 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -62,7 +62,8 @@ core_param(novmcoredd, vmcoredd_disabled, bool, 0);
>  /* Device Dump Size */
>  static size_t vmcoredd_orig_sz;
>  
> -static DEFINE_SPINLOCK(vmcore_cb_lock);
> +static DEFINE_MUTEX(vmcore_mutex);
> +
>  DEFINE_STATIC_SRCU(vmcore_cb_srcu);
>  /* List of registered vmcore callbacks. */
>  static LIST_HEAD(vmcore_cb_list);
> @@ -72,7 +73,7 @@ static bool vmcore_opened;
>  void register_vmcore_cb(struct vmcore_cb *cb)
>  {
>  	INIT_LIST_HEAD(&cb->next);
> -	spin_lock(&vmcore_cb_lock);
> +	mutex_lock(&vmcore_mutex);
>  	list_add_tail(&cb->next, &vmcore_cb_list);
>  	/*
>  	 * Registering a vmcore callback after the vmcore was opened is
> @@ -80,13 +81,13 @@ void register_vmcore_cb(struct vmcore_cb *cb)
>  	 */
>  	if (vmcore_opened)
>  		pr_warn_once("Unexpected vmcore callback registration\n");
> -	spin_unlock(&vmcore_cb_lock);
> +	mutex_unlock(&vmcore_mutex);
>  }
>  EXPORT_SYMBOL_GPL(register_vmcore_cb);
>  
>  void unregister_vmcore_cb(struct vmcore_cb *cb)
>  {
> -	spin_lock(&vmcore_cb_lock);
> +	mutex_lock(&vmcore_mutex);
>  	list_del_rcu(&cb->next);
>  	/*
>  	 * Unregistering a vmcore callback after the vmcore was opened is
> @@ -95,7 +96,7 @@ void unregister_vmcore_cb(struct vmcore_cb *cb)
>  	 */
>  	if (vmcore_opened)
>  		pr_warn_once("Unexpected vmcore callback unregistration\n");
> -	spin_unlock(&vmcore_cb_lock);
> +	mutex_unlock(&vmcore_mutex);
>  
>  	synchronize_srcu(&vmcore_cb_srcu);
>  }
> @@ -120,9 +121,9 @@ static bool pfn_is_ram(unsigned long pfn)
>  
>  static int open_vmcore(struct inode *inode, struct file *file)
>  {
> -	spin_lock(&vmcore_cb_lock);
> +	mutex_lock(&vmcore_mutex);
>  	vmcore_opened = true;
> -	spin_unlock(&vmcore_cb_lock);
> +	mutex_unlock(&vmcore_mutex);
>  
>  	return 0;
>  }
> -- 
> 2.46.1
>