Hi,
On Tue, Oct 11, 2022 at 09:06:33AM +0200, Takashi Iwai wrote:
> On Wed, 21 Sep 2022 09:34:30 +0200,
> Takashi Iwai wrote:
> >
> > On Thu, 08 Sep 2022 15:27:54 +0200,
> > Takashi Iwai wrote:
> > >
> > > The dvb-core tries to sync the releases of opened files at
> > > dvb_dmxdev_release() with two refcounts: dvbdev->users and
> > > dvr_dvbdev->users. A problem is present in those two syncs: when yet
> > > another dvb_demux_open() is called during those sync waits,
> > > dvb_demux_open() continues to process even if the device is being
> > > closed. This includes the increment of the former refcount, resulting
> > > in the leftover refcount after the sync of the latter refcount at
> > > dvb_dmxdev_release(). It ends up with use-after-free, since the
> > > function believes that all usages were gone and releases the
> > > resources.
> > >
> > > This patch addresses the problem by adding the check of dmxdev->exit
> > > flag at dvb_demux_open(), just like dvb_dvr_open() already does. With
> > > the exit flag check, the second call of dvb_demux_open() fails, hence
> > > the further corruption can be avoided.
> > >
> > > Also for avoiding the races of the dmxdev->exit flag reference, this
> > > patch serializes the dmxdev->exit set up and the sync waits with the
> > > dmxdev->mutex lock at dvb_dmxdev_release(). Without the mutex lock,
> > > dvb_demux_open() (or dvb_dvr_open()) may run concurrently with
> > > dvb_dmxdev_release(), which allows to skip the exit flag check and
> > > continue the open process that is being closed.
> > >
> > > Reported-by: Hyunwoo Kim <imv4bel@xxxxxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> >
> > Any review on this?
> >
> > FWIW, now CVE-2022-41218 is assigned for those bugs as a security
> > issue.
>
> A gentle ping again.
>
> Or if any other fix for this security issue is already available,
> please let me know.
is this correct, the fix is yet missing (or was it fixed by other
means?).
Regards,
Salvatore
> >
> > > ---
> > > drivers/media/dvb-core/dmxdev.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> > > index f6ee678107d3..9ce5f010de3f 100644
> > > --- a/drivers/media/dvb-core/dmxdev.c
> > > +++ b/drivers/media/dvb-core/dmxdev.c
> > > @@ -790,6 +790,11 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
> > > if (mutex_lock_interruptible(&dmxdev->mutex))
> > > return -ERESTARTSYS;
> > >
> > > + if (dmxdev->exit) {
> > > + mutex_unlock(&dmxdev->mutex);
> > > + return -ENODEV;
> > > + }
> > > +
> > > for (i = 0; i < dmxdev->filternum; i++)
> > > if (dmxdev->filter[i].state == DMXDEV_STATE_FREE)
> > > break;
> > > @@ -1448,7 +1453,10 @@ EXPORT_SYMBOL(dvb_dmxdev_init);
> > >
> > > void dvb_dmxdev_release(struct dmxdev *dmxdev)
> > > {
> > > + mutex_lock(&dmxdev->mutex);
> > > dmxdev->exit = 1;
> > > + mutex_unlock(&dmxdev->mutex);
> > > +
> > > if (dmxdev->dvbdev->users > 1) {
> > > wait_event(dmxdev->dvbdev->wait_queue,
> > > dmxdev->dvbdev->users == 1);
> > > --
> > > 2.35.3
> > >
>
