netdev - Re: [PATCHv3 1/2] vhost: return task creation error instead of NULL

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] [day] [month] [year] [list]

Message-ID: <e04b172f-f137-4d9a-a99d-e757088df4cf@oracle.com>
Date: Fri, 28 Feb 2025 12:34:53 -0600
From: Mike Christie <michael.christie@...cle.com>
To: Keith Busch <kbusch@...a.com>, seanjc@...gle.com, pbonzini@...hat.com,
        kvm@...r.kernel.org
Cc: leiyang@...hat.com, virtualization@...ts.linux.dev, x86@...nel.org,
        netdev@...r.kernel.org, Keith Busch <kbusch@...nel.org>
Subject: Re: [PATCHv3 1/2] vhost: return task creation error instead of NULL

On 2/27/25 5:06 PM, Keith Busch wrote:
> From: Keith Busch <kbusch@...nel.org>
> 
> Lets callers distinguish why the vhost task creation failed. No one
> currently cares why it failed, so no real runtime change from this
> patch, but that will not be the case for long.
> 
> Signed-off-by: Keith Busch <kbusch@...nel.org>
> ---
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  drivers/vhost/vhost.c  | 2 +-
>  kernel/vhost_task.c    | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index d4ac4a1f8b81b..18ca1ea6dc240 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7471,7 +7471,7 @@ static void kvm_mmu_start_lpage_recovery(struct once *once)
>  				      kvm_nx_huge_page_recovery_worker_kill,
>  				      kvm, "kvm-nx-lpage-recovery");
>  
> -	if (!nx_thread)
> +	if (IS_ERR(nx_thread))
>  		return;
>  
>  	vhost_task_start(nx_thread);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 9ac25d08f473e..63612faeab727 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -666,7 +666,7 @@ static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
>  
>  	vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
>  				 worker, name);
> -	if (!vtsk)
> +	if (IS_ERR(vtsk))
>  		goto free_worker;
>  
>  	mutex_init(&worker->mutex);
> diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
> index 8800f5acc0071..2ef2e1b800916 100644
> --- a/kernel/vhost_task.c
> +++ b/kernel/vhost_task.c
> @@ -133,7 +133,7 @@ struct vhost_task *vhost_task_create(bool (*fn)(void *),
>  
>  	vtsk = kzalloc(sizeof(*vtsk), GFP_KERNEL);
>  	if (!vtsk)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  	init_completion(&vtsk->exited);
>  	mutex_init(&vtsk->exit_mutex);
>  	vtsk->data = arg;
> @@ -145,7 +145,7 @@ struct vhost_task *vhost_task_create(bool (*fn)(void *),
>  	tsk = copy_process(NULL, 0, NUMA_NO_NODE, &args);
>  	if (IS_ERR(tsk)) {
>  		kfree(vtsk);
> -		return NULL;
> +		return ERR_PTR(PTR_ERR(tsk));
>  	}
>  
>  	vtsk->task = tsk;


The vhost task parts look ok to me.

Reviewed-by: Mike Christie <michael.christie@...cle.com>