* Re: [PATCH 6.11 066/184] media: dvbdev: prevent the risk of out of memory access [not found] ` <20241112101903.395286793@linuxfoundation.org> @ 2024-11-18 0:33 ` Nathan Chancellor 2024-11-19 12:03 ` Greg Kroah-Hartman 0 siblings, 1 reply; 2+ messages in thread From: Nathan Chancellor @ 2024-11-18 0:33 UTC (permalink / raw) To: Greg Kroah-Hartman, Sasha Levin Cc: stable, patches, Mauro Carvalho Chehab, llvm 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@kernel.org> > > [ 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@kernel.org> > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > 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 > > > ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 6.11 066/184] media: dvbdev: prevent the risk of out of memory access 2024-11-18 0:33 ` [PATCH 6.11 066/184] media: dvbdev: prevent the risk of out of memory access Nathan Chancellor @ 2024-11-19 12:03 ` Greg Kroah-Hartman 0 siblings, 0 replies; 2+ messages in thread From: Greg Kroah-Hartman @ 2024-11-19 12:03 UTC (permalink / raw) To: Nathan Chancellor Cc: Sasha Levin, stable, patches, Mauro Carvalho Chehab, llvm 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@kernel.org> > > > > [ 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@kernel.org> > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > --- > > 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 ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-11-19 12:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241112101900.865487674@linuxfoundation.org>
[not found] ` <20241112101903.395286793@linuxfoundation.org>
2024-11-18 0:33 ` [PATCH 6.11 066/184] media: dvbdev: prevent the risk of out of memory access Nathan Chancellor
2024-11-19 12:03 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox