Received-SPF: pass (google.com: domain of linux-kernel+bounces-75746-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Thu, 22 Feb 2024 08:57:44 +0800
From: Kent Gibson <warthog618@gmail.com>
To: Herve Codina <herve.codina@bootlin.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Luca Ceresoli <luca.ceresoli@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH 2/2] gpiolib: cdev: release IRQs when the gpio chip
 device is removed
Message-ID: <20240222005744.GA3603@rigel>
References: <20240220111019.133697-1-herve.codina@bootlin.com>
 <20240220111019.133697-3-herve.codina@bootlin.com>
 <20240220142959.GA244726@rigel>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240220142959.GA244726@rigel>

On Tue, Feb 20, 2024 at 10:29:59PM +0800, Kent Gibson wrote:
> On Tue, Feb 20, 2024 at 12:10:18PM +0100, Herve Codina wrote:

..

> >  }
> >
> > +static int linereq_unregistered_notify(struct notifier_block *nb,
> > +				       unsigned long action, void *data)
> > +{
> > +	struct linereq *lr = container_of(nb, struct linereq,
> > +					  device_unregistered_nb);
> > +	int i;
> > +
> > +	for (i = 0; i < lr->num_lines; i++) {
> > +		if (lr->lines[i].desc)
> > +			edge_detector_stop(&lr->lines[i]);
> > +	}
> > +
>
> Firstly, the re-ordering in the previous patch creates a race,
> as the NULLing of the gdev->chip serves to numb the cdev ioctls, so
> there is now a window between the notifier being called and that numbing,
> during which userspace may call linereq_set_config() and re-request
> the irq.
>
> There is also a race here with linereq_set_config().  That can be prevented
> by holding the lr->config_mutex - assuming the notifier is not being called
> from atomic context.
>

It occurs to me that the fixed reordering in patch 1 would place
the notifier call AFTER the NULLing of the ioctls, so there will no longer
be any chance of a race with linereq_set_config() - so holding the
config_mutex semaphore is not necessary.

In which case this patch is fine - it is only patch 1 that requires
updating.

Cheers,
Kent.