On Sun, Nov 17, 2024 at 05:33:38PM -0700, Nathan Chancellor wrote:
> On Tue, Nov 12, 2024 at 11:20:24AM +0100, Greg Kroah-Hartman wrote:
> > 6.11-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
> >
> > [ Upstream commit 972e63e895abbe8aa1ccbdbb4e6362abda7cd457 ]
> >
> > The dvbdev contains a static variable used to store dvb minors.
> >
> > The behavior of it depends if CONFIG_DVB_DYNAMIC_MINORS is set
> > or not. When not set, dvb_register_device() won't check for
> > boundaries, as it will rely that a previous call to
> > dvb_register_adapter() would already be enforcing it.
> >
> > On a similar way, dvb_device_open() uses the assumption
> > that the register functions already did the needed checks.
> >
> > This can be fragile if some device ends using different
> > calls. This also generate warnings on static check analysers
> > like Coverity.
> >
> > So, add explicit guards to prevent potential risk of OOM issues.
> >
> > Fixes: 5dd3f3071070 ("V4L/DVB (9361): Dynamic DVB minor allocation")
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> > ---
> > drivers/media/dvb-core/dvbdev.c | 17 +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> > index b43695bc51e75..14f323fbada71 100644
> > --- a/drivers/media/dvb-core/dvbdev.c
> > +++ b/drivers/media/dvb-core/dvbdev.c
> > @@ -86,10 +86,15 @@ static DECLARE_RWSEM(minor_rwsem);
> > static int dvb_device_open(struct inode *inode, struct file *file)
> > {
> > struct dvb_device *dvbdev;
> > + unsigned int minor = iminor(inode);
> > +
> > + if (minor >= MAX_DVB_MINORS)
> > + return -ENODEV;
> >
> > mutex_lock(&dvbdev_mutex);
> > down_read(&minor_rwsem);
> > - dvbdev = dvb_minors[iminor(inode)];
> > +
> > + dvbdev = dvb_minors[minor];
> >
> > if (dvbdev && dvbdev->fops) {
> > int err = 0;
> > @@ -525,7 +530,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
> > for (minor = 0; minor < MAX_DVB_MINORS; minor++)
> > if (!dvb_minors[minor])
> > break;
> > - if (minor == MAX_DVB_MINORS) {
> > + if (minor >= MAX_DVB_MINORS) {
> > if (new_node) {
> > list_del(&new_node->list_head);
> > kfree(dvbdevfops);
> > @@ -540,6 +545,14 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
> > }
> > #else
> > minor = nums2minor(adap->num, type, id);
> > + if (minor >= MAX_DVB_MINORS) {
> > + dvb_media_device_free(dvbdev);
> > + list_del(&dvbdev->list_head);
> > + kfree(dvbdev);
> > + *pdvbdev = NULL;
> > + mutex_unlock(&dvbdev_register_lock);
> > + return ret;
>
> This needs commit a4aebaf6e6ef ("media: dvbdev: fix the logic when
> DVB_DYNAMIC_MINORS is not set"), otherwise there is a warning with
> certain configurations when building with clang:
>
> drivers/media/dvb-core/dvbdev.c:554:10: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> 554 | return ret;
> | ^~~
> drivers/media/dvb-core/dvbdev.c:463:13: note: initialize the variable 'ret' to silence this warning
> 463 | int id, ret;
> | ^
> | = 0
> 1 warning generated.
>
> I was somewhat surprised when this warning showed up in my stable
> builds, until I realized that change does not have a Fixes tag like it
> really should have...
Now queued up, thanks.
greg k-h
