From: Dan Carpenter <dan.carpenter@linaro.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Pavan Kumar Yalagada <pavankumaryalagada@gmail.com>,
parthiban.veerasooran@microchip.com,
christian.gromm@microchip.com,
laurent.pinchart+renesas@ideasonboard.com,
hverkuil+cisco@kernel.org, linux-staging@lists.linux.dev,
linux-media@vger.kernel.org
Subject: Re: [PATCH v5] staging: most: video: prevent probes during component exit
Date: Wed, 3 Dec 2025 12:31:56 +0300 [thread overview]
Message-ID: <aTADjJE99n2ju_kJ@stanley.mountain> (raw)
In-Reply-To: <2025120308-unlucky-chevron-0071@gregkh>
On Wed, Dec 03, 2025 at 10:05:17AM +0100, Greg KH wrote:
> On Wed, Dec 03, 2025 at 03:34:11AM -0500, Pavan Kumar Yalagada wrote:
> > When comp_exit() runs, comp_probe_channel() could still add new devices
> > to video_devices, creating a race and potentially leaving the list in
> > an inconsistent state.
>
> Wait, please step back. How can this happen? I would argue that if it
> could happen, then this must be solved at the higher level. The most
> core should not be allowing this, as you do not want to be forced to
> solve this for each individual driver (hint, the driver core does not
> allow this to happen for normal driver operations for this very reason.)
>
> You are only solving this for one most driver, not all of them (well,
> there are only two with this codepath...) Please move back up the stack
> a bit and prevent this from even being an issue at all. Especially as I
> do not think this really solves the problem, see below:
>
I had looked at this before and had assumed it's more complicated than
it actually is. All this stuff comes from configfs so all we need to do
is move the most_deregister_configfs_subsys(&comp) earlier in the
function.
> > This patch prevents new devices from being added while exiting by:
> >
> > - comp_probe_channel() checks comp_exiting before modifying video_devices.
> >
> > if (comp_exiting) {
> > spin_unlock_irq(&list_lock);
> > ret = -BUSY;
> > goto err_unreg;
> > }
>
> That's great, but what prevents comp_exiting from changing right after
> you check it?
That would have been okay. In that case it would have been added and then
removed from the list.
But, you're right. It's not the right fix.
regards,
dan carpenter
next prev parent reply other threads:[~2025-12-03 9:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-03 8:34 [PATCH v5] staging: most: video: prevent probes during component exit Pavan Kumar Yalagada
2025-12-03 9:05 ` Greg KH
2025-12-03 9:31 ` Dan Carpenter [this message]
2025-12-03 9:13 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aTADjJE99n2ju_kJ@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=christian.gromm@microchip.com \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil+cisco@kernel.org \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=parthiban.veerasooran@microchip.com \
--cc=pavankumaryalagada@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).