[PATCH] media: dvb-core: roll back partial PES feed starts

meihaipeng posted 1 patch 1 week, 5 days ago
drivers/media/dvb-core/dmxdev.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
[PATCH] media: dvb-core: roll back partial PES feed starts
Posted by meihaipeng 1 week, 5 days ago
syzbot reported a kmemleak in vidtv PSI descriptor allocation paths, but
the leak is caused by dmxdev leaving partially started PES feeds running
after a later PID start fails.

dvb_dmxdev_filter_start() keeps the filter in DMXDEV_STATE_SET until all
PIDs have been started successfully. That is true both for the initial
multi-PID start after several DMX_ADD_PID calls and for a later DMX_START
restart of an already running filter. If one PID has already started and
a later one fails, the error path calls dvb_dmxdev_filter_stop(), but
that helper returns immediately while the filter is still in
DMXDEV_STATE_SET. The already started feeds are then left alive, and
release/close paths can repeat the same no-op stop before dropping the
PID list.

Fix this by clearing feed->ts after start failures, rolling back a PID
that failed an immediate DMX_ADD_PID start, and explicitly stopping any
PES feeds that were started before dvb_dmxdev_filter_start() aborts,
regardless of whether it is the first multi-PID start or a restart.

Fixes: 1cb662a314499 ("V4L/DVB (12275): Add two new ioctls: DMX_ADD_PID and DMX_REMOVE_PID")
Reported-by: syzbot+acc3b75c010446ad403f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=acc3b75c010446ad403f
Signed-off-by: meihaipeng <meihaipeng@uniontech.com>

---
 drivers/media/dvb-core/dmxdev.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 3c8bc75e4d6c..b5209e1611ad 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -612,6 +612,21 @@ static inline int dvb_dmxdev_filter_reset(struct dmxdev_filter *dmxdevfilter)
 	return 0;
 }
 
+static void dvb_dmxdev_cleanup_pes(struct dmxdev_filter *dmxdevfilter)
+{
+	struct dmxdev_feed *feed;
+	struct dmx_demux *demux = dmxdevfilter->dev->demux;
+
+	list_for_each_entry(feed, &dmxdevfilter->feed.ts, next) {
+		if (!feed->ts)
+			continue;
+
+		feed->ts->stop_filtering(feed->ts);
+		demux->release_ts_feed(demux, feed->ts);
+		feed->ts = NULL;
+	}
+}
+
 static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
 				 struct dmxdev_filter *filter,
 				 struct dmxdev_feed *feed)
@@ -652,12 +667,14 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
 	ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
 	if (ret < 0) {
 		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
+		feed->ts = NULL;
 		return ret;
 	}
 
 	ret = tsfeed->start_filtering(tsfeed);
 	if (ret < 0) {
 		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
+		feed->ts = NULL;
 		return ret;
 	}
 
@@ -768,7 +785,8 @@ static int dvb_dmxdev_filter_start(struct dmxdev_filter *filter)
 		list_for_each_entry(feed, &filter->feed.ts, next) {
 			ret = dvb_dmxdev_start_feed(dmxdev, filter, feed);
 			if (ret < 0) {
-				dvb_dmxdev_filter_stop(filter);
+				dvb_dmxdev_cleanup_pes(filter);
+				dvb_ringbuffer_flush(&filter->buffer);
 				return ret;
 			}
 		}
@@ -884,6 +902,7 @@ static int dvb_dmxdev_add_pid(struct dmxdev *dmxdev,
 			      struct dmxdev_filter *filter, u16 pid)
 {
 	struct dmxdev_feed *feed;
+	int ret;
 
 	if ((filter->type != DMXDEV_TYPE_PES) ||
 	    (filter->state < DMXDEV_STATE_SET))
@@ -901,8 +920,14 @@ static int dvb_dmxdev_add_pid(struct dmxdev *dmxdev,
 	feed->pid = pid;
 	list_add(&feed->next, &filter->feed.ts);
 
-	if (filter->state >= DMXDEV_STATE_GO)
-		return dvb_dmxdev_start_feed(dmxdev, filter, feed);
+	if (filter->state >= DMXDEV_STATE_GO) {
+		ret = dvb_dmxdev_start_feed(dmxdev, filter, feed);
+		if (ret < 0) {
+			list_del(&feed->next);
+			kfree(feed);
+		}
+		return ret;
+	}
 
 	return 0;
 }
-- 
2.20.1