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...
Cheers,
Nathan
> + }
> #endif
> dvbdev->minor = minor;
> dvb_minors[minor] = dvb_device_get(dvbdev);
> --
> 2.43.0
>
>
>
