Date: Fri, 20 Nov 2020 13:34:30 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: "Wall, Stephen" <stephen.wall@redcom.com> Cc: "freebsd-drivers@freebsd.org" <freebsd-drivers@freebsd.org> Subject: Re: bug in sound driver? Message-ID: <X7epxqZFFzH75no5@kib.kiev.ua> In-Reply-To: <DM6PR09MB4807F15897B9571B9D824D78EEE00@DM6PR09MB4807.namprd09.prod.outlook.com> References: <DM6PR09MB4807F15897B9571B9D824D78EEE00@DM6PR09MB4807.namprd09.prod.outlook.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Nov 19, 2020 at 11:24:13PM +0000, Wall, Stephen wrote:
> I am working on a driver that is a modified copy of the sound driver, and I've found something that might be a bug.
>
> In sys/dev/sound/pcm/dsp.c, the funcion dap_io_ops() (called by dsp_read() and dsp_write()) calls getchns() to get pointers to the channel control structures. getchns() also locks the mutexs on those channels.
>
> After calling getchns(), dsp_io_ops() checks to see if a NULL pointer was returns, OR if channels flags field does not have CHN_F_BUSY set. If either of those are true, it release Giant and exits. That's fine if the pointer is NULL, since the lock wouldn't have happened but if the BUSY flag is not set, then dsp_io_ops() (and dsp_read/write()) returns with the channel still locked.
>
> In my driver, I am seeing kernel panics in some situations with this message as a result:
> Sleeping thread (tid xxx, pid yyy) owns a non-sleepable lock
>
> Is this a bug in the sound driver? Should it check these two conditions separately and call relchns() if it fails due to the BUSY flag?
>
I think you are right, please check that the following fixes the issue:
diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 4df035f99c8..0593a585b0f 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -857,6 +857,8 @@ dsp_io_ops(struct cdev *i_dev, struct uio *buf)
getchns(i_dev, &rdch, &wrch, prio);
if (*ch == NULL || !((*ch)->flags & CHN_F_BUSY)) {
+ if (rdch != NULL || wrch != NULL)
+ relchns(i_dev, rdch, wrch, prio);
PCM_GIANT_EXIT(d);
return (EBADF);
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?X7epxqZFFzH75no5>