netdev - RE: [PATCH 3/7] irqbypass: Take ownership of producer/consumer token tracking

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: <BN9PR11MB5276385B4F4DB1919D4908CF8CB72@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Thu, 10 Apr 2025 07:28:34 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Sean Christopherson <seanjc@...gle.com>, "Michael S. Tsirkin"
	<mst@...hat.com>, Jason Wang <jasowang@...hat.com>, Paolo Bonzini
	<pbonzini@...hat.com>, Alex Williamson <alex.williamson@...hat.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Oliver Upton
	<oliver.upton@...ux.dev>, David Matlack <dmatlack@...gle.com>, Like Xu
	<like.xu.linux@...il.com>, Yong He <alexyonghe@...cent.com>
Subject: RE: [PATCH 3/7] irqbypass: Take ownership of producer/consumer token
 tracking

> From: Sean Christopherson <seanjc@...gle.com>
> Sent: Saturday, April 5, 2025 5:15 AM
> 
> @@ -505,15 +505,12 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_core_device *vdev,
>  	if (ret)
>  		goto out_put_eventfd_ctx;
> 
> -	ctx->producer.token = trigger;
>  	ctx->producer.irq = irq;
> -	ret = irq_bypass_register_producer(&ctx->producer);
> +	ret = irq_bypass_register_producer(&ctx->producer, trigger);
>  	if (unlikely(ret)) {
>  		dev_info(&pdev->dev,
>  		"irq bypass producer (token %p) registration fails: %d\n",
>  		ctx->producer.token, ret);

Use 'trigger' as ctx->producer.token is NULL if registration fails. 

> @@ -18,20 +19,20 @@ struct irq_bypass_consumer;
>   * The IRQ bypass manager is a simple set of lists and callbacks that allows
>   * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
>   * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> - * via a shared token (ex. eventfd_ctx).  Producers and consumers register
> - * independently.  When a token match is found, the optional @stop
> callback
> - * will be called for each participant.  The pair will then be connected via
> - * the @add_* callbacks, and finally the optional @start callback will allow
> - * any final coordination.  When either participant is unregistered, the
> - * process is repeated using the @del_* callbacks in place of the @add_*
> - * callbacks.  Match tokens must be unique per producer/consumer, 1:N
> pairings
> - * are not supported.
> + * via a shared eventfd_ctx).  Producers and consumers register
> independently.

s/eventfd_ctx)/eventfd_ctx/

> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer,
> +				 struct eventfd_ctx *eventfd)
>  {
>  	struct irq_bypass_consumer *tmp;
>  	struct irq_bypass_producer *producer;
>  	int ret;
> 
> -	if (!consumer->token ||
> -	    !consumer->add_producer || !consumer->del_producer)
> +	if (WARN_ON_ONCE(consumer->token))
> +		return -EINVAL;
> +
> +	if (!consumer->add_producer || !consumer->del_producer)
>  		return -EINVAL;
> 
>  	mutex_lock(&lock);
> 
>  	list_for_each_entry(tmp, &consumers, node) {
> -		if (tmp->token == consumer->token || tmp == consumer) {
> +		if (tmp->token == eventfd || tmp == consumer) {
>  			ret = -EBUSY;
>  			goto out_err;
>  		}
>  	}

the 2nd check 'tmp == consumer' is redundant. If they are equal 
consumer->token is not NULL then the earlier WARN_ON will be
triggered already.

otherwise,

Reviewed-by: Kevin Tian <kevin.tian@...el.com>