[PATCH 0/3] staging: media: av7110: check demux and net initialization in av7110_register()

[PATCH 0/3] staging: media: av7110: check demux and net initialization in av7110_register()

[Andrey Karelin]

av7110_register() ignores the return values of dvb_dmx_init(),
dvb_dmxdev_init() and dvb_net_init() for both the main and the optional
budget-patch demux. On failure probing continues with half-initialized
objects, which leads to NULL pointer dereferences on first use or on
device removal. av7110_attach() does not call dvb_unregister() when
av7110_register() fails, so with this series av7110_register() unwinds
what it has set up on its own.

Andrey Karelin (3):
  staging: media: av7110: check dvb_dmx_init() result in
    av7110_register()
  staging: media: av7110: check dvb_dmxdev_init() result in
    av7110_register()
  staging: media: av7110: check dvb_net_init() result in
    av7110_register()

 drivers/staging/media/av7110/av7110.c | 49 ++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 9 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0

[PATCH 1/3] staging: media: av7110: check dvb_dmx_init() result in av7110_register()

[Andrey Karelin]

av7110_register() ignores the return value of dvb_dmx_init() for both
the main demux and the optional budget-patch demux1. dvb_dmx_init()
fails if it cannot allocate the filter or feed arrays and leaves them
NULL, but probing continues, and the first filter or feed allocation on
that demux, e.g. when userspace sets a filter on the demux device,
dereferences the NULL array.

Check the result and fail the registration. av7110_attach() does not
call dvb_unregister() when av7110_register() fails, so add an error path
that releases everything av7110_register() has set up so far and clears
the registered flag. Use it for the existing add_frontend() and
connect_frontend() error returns as well, which leaked the demux and the
dmxdev.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Andrey Karelin <a.karelin@ideco.ru>
---
 drivers/staging/media/av7110/av7110.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/av7110/av7110.c b/drivers/staging/media/av7110/av7110.c
index 432404eceaea..ecc15bb222c5 100644
--- a/drivers/staging/media/av7110/av7110.c
+++ b/drivers/staging/media/av7110/av7110.c
@@ -1274,7 +1274,9 @@ static int av7110_register(struct av7110 *av7110)
 	dvbdemux->dmx.capabilities = (DMX_TS_FILTERING | DMX_SECTION_FILTERING |
 				      DMX_MEMORY_BASED_FILTERING);
 
-	dvb_dmx_init(&av7110->demux);
+	ret = dvb_dmx_init(&av7110->demux);
+	if (ret < 0)
+		goto err_registered;
 	av7110->demux.dmx.get_stc = dvb_get_stc;
 
 	av7110->dmxdev.filternum = (av7110->full_ts) ? 256 : 32;
@@ -1288,19 +1290,19 @@ static int av7110_register(struct av7110 *av7110)
 	ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &av7110->hw_frontend);
 
 	if (ret < 0)
-		return ret;
+		goto err_dmxdev_release;
 
 	av7110->mem_frontend.source = DMX_MEMORY_FE;
 
 	ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &av7110->mem_frontend);
 
 	if (ret < 0)
-		return ret;
+		goto err_dmxdev_release;
 
 	ret = dvbdemux->dmx.connect_frontend(&dvbdemux->dmx,
 					     &av7110->hw_frontend);
 	if (ret < 0)
-		return ret;
+		goto err_dmxdev_release;
 
 	av7110_av_register(av7110);
 	av7110_ca_register(av7110);
@@ -1327,7 +1329,9 @@ static int av7110_register(struct av7110 *av7110)
 		dvbdemux1->dmx.capabilities = (DMX_TS_FILTERING | DMX_SECTION_FILTERING |
 					       DMX_MEMORY_BASED_FILTERING);
 
-		dvb_dmx_init(&av7110->demux1);
+		ret = dvb_dmx_init(&av7110->demux1);
+		if (ret < 0)
+			goto err_unregister;
 
 		av7110->dmxdev1.filternum = 256;
 		av7110->dmxdev1.demux = &dvbdemux1->dmx;
@@ -1339,6 +1343,18 @@ static int av7110_register(struct av7110 *av7110)
 		pr_info("additional demux1 for budget-patch registered\n");
 	}
 	return 0;
+
+err_unregister:
+	dvb_net_release(&av7110->dvb_net);
+	dvb_unregister_device(av7110->osd_dev);
+	av7110_ca_unregister(av7110);
+	av7110_av_unregister(av7110);
+err_dmxdev_release:
+	dvb_dmxdev_release(&av7110->dmxdev);
+	dvb_dmx_release(&av7110->demux);
+err_registered:
+	av7110->registered = 0;
+	return ret;
 }
 
 static void dvb_unregister(struct av7110 *av7110)
-- 
2.56.0

[PATCH 2/3] staging: media: av7110: check dvb_dmxdev_init() result in av7110_register()

[Andrey Karelin]

av7110_register() ignores the return value of dvb_dmxdev_init() for both
the main dmxdev and the optional budget-patch dmxdev1. dvb_dmxdev_init()
fails if the demux cannot be opened, the filter array cannot be
allocated or the demux/dvr device cannot be registered. Probing then
continues, and on device removal dvb_unregister() calls
dvb_dmxdev_release(), which dereferences dmxdev->dvbdev and
dmxdev->dvr_dvbdev that are NULL or, if only the dvr device failed to
register, already freed.

Check the result and unwind on failure, releasing the corresponding
demux as well.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Andrey Karelin <a.karelin@ideco.ru>
---
 drivers/staging/media/av7110/av7110.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/av7110/av7110.c b/drivers/staging/media/av7110/av7110.c
index ecc15bb222c5..3371e33db103 100644
--- a/drivers/staging/media/av7110/av7110.c
+++ b/drivers/staging/media/av7110/av7110.c
@@ -1283,7 +1283,9 @@ static int av7110_register(struct av7110 *av7110)
 	av7110->dmxdev.demux = &dvbdemux->dmx;
 	av7110->dmxdev.capabilities = 0;
 
-	dvb_dmxdev_init(&av7110->dmxdev, &av7110->dvb_adapter);
+	ret = dvb_dmxdev_init(&av7110->dmxdev, &av7110->dvb_adapter);
+	if (ret < 0)
+		goto err_dmx_release;
 
 	av7110->hw_frontend.source = DMX_FRONTEND_0;
 
@@ -1337,13 +1339,17 @@ static int av7110_register(struct av7110 *av7110)
 		av7110->dmxdev1.demux = &dvbdemux1->dmx;
 		av7110->dmxdev1.capabilities = 0;
 
-		dvb_dmxdev_init(&av7110->dmxdev1, &av7110->dvb_adapter);
+		ret = dvb_dmxdev_init(&av7110->dmxdev1, &av7110->dvb_adapter);
+		if (ret < 0)
+			goto err_dmx1_release;
 
 		dvb_net_init(&av7110->dvb_adapter, &av7110->dvb_net1, &dvbdemux1->dmx);
 		pr_info("additional demux1 for budget-patch registered\n");
 	}
 	return 0;
 
+err_dmx1_release:
+	dvb_dmx_release(&av7110->demux1);
 err_unregister:
 	dvb_net_release(&av7110->dvb_net);
 	dvb_unregister_device(av7110->osd_dev);
@@ -1351,6 +1357,7 @@ static int av7110_register(struct av7110 *av7110)
 	av7110_av_unregister(av7110);
 err_dmxdev_release:
 	dvb_dmxdev_release(&av7110->dmxdev);
+err_dmx_release:
 	dvb_dmx_release(&av7110->demux);
 err_registered:
 	av7110->registered = 0;
-- 
2.56.0

[PATCH 3/3] staging: media: av7110: check dvb_net_init() result in av7110_register()

[Andrey Karelin]

av7110_register() ignores the return value of dvb_net_init() for both
dvb_net and the optional budget-patch dvb_net1. dvb_net_init() fails if
the net device cannot be registered and leaves dvbnet->dvbdev NULL.
Probing then continues, and on device removal dvb_unregister() calls
dvb_net_release(), which dereferences dvbnet->dvbdev.

Check the result and unwind on failure.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Andrey Karelin <a.karelin@ideco.ru>
---
 drivers/staging/media/av7110/av7110.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/av7110/av7110.c b/drivers/staging/media/av7110/av7110.c
index 3371e33db103..9390c347da50 100644
--- a/drivers/staging/media/av7110/av7110.c
+++ b/drivers/staging/media/av7110/av7110.c
@@ -1314,7 +1314,9 @@ static int av7110_register(struct av7110 *av7110)
 			    &dvbdev_osd, av7110, DVB_DEVICE_OSD, 0);
 #endif
 
-	dvb_net_init(&av7110->dvb_adapter, &av7110->dvb_net, &dvbdemux->dmx);
+	ret = dvb_net_init(&av7110->dvb_adapter, &av7110->dvb_net, &dvbdemux->dmx);
+	if (ret < 0)
+		goto err_unregister_devices;
 
 	if (budgetpatch) {
 		/* initialize software demux1 without its own frontend
@@ -1333,7 +1335,7 @@ static int av7110_register(struct av7110 *av7110)
 
 		ret = dvb_dmx_init(&av7110->demux1);
 		if (ret < 0)
-			goto err_unregister;
+			goto err_net_release;
 
 		av7110->dmxdev1.filternum = 256;
 		av7110->dmxdev1.demux = &dvbdemux1->dmx;
@@ -1343,15 +1345,21 @@ static int av7110_register(struct av7110 *av7110)
 		if (ret < 0)
 			goto err_dmx1_release;
 
-		dvb_net_init(&av7110->dvb_adapter, &av7110->dvb_net1, &dvbdemux1->dmx);
+		ret = dvb_net_init(&av7110->dvb_adapter, &av7110->dvb_net1,
+				   &dvbdemux1->dmx);
+		if (ret < 0)
+			goto err_dmxdev1_release;
 		pr_info("additional demux1 for budget-patch registered\n");
 	}
 	return 0;
 
+err_dmxdev1_release:
+	dvb_dmxdev_release(&av7110->dmxdev1);
 err_dmx1_release:
 	dvb_dmx_release(&av7110->demux1);
-err_unregister:
+err_net_release:
 	dvb_net_release(&av7110->dvb_net);
+err_unregister_devices:
 	dvb_unregister_device(av7110->osd_dev);
 	av7110_ca_unregister(av7110);
 	av7110_av_unregister(av7110);
-- 
2.56.0