Forwarded: [PATCH 2/2] media: vidtv: fix error handling in channel SI init functions

[syzbot <syzbot+acc3b75c010446ad403f@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH 2/2] media: vidtv: fix error handling in channel SI init functions
Author: zhanghaotian@uniontech.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

Several functions in vidtv_channel.c have error paths that can lead to
memory leaks or use-after-free when vidtv_psi_desc_clone() fails:

1. vidtv_channel_sdt_serv_cat_into_new(): passes the accumulated "tail"
   pointer to vidtv_psi_sdt_service_init() which chains the new service
   before vidtv_psi_desc_clone() is called.  If cloning then fails, the
   "free_tail" error path destroys tail while head->next still points
   to the freed memory, causing a use-after-free when "free" later
   destroys head.

2. vidtv_channel_eit_event_cat_into_new(): silently ignores a NULL
   return from vidtv_psi_desc_clone(), creating an EIT event with no
   descriptor.

3. vidtv_channel_pmt_match_sections(): silently ignores a NULL return
   from vidtv_psi_desc_clone(), creating a PMT stream with no
   descriptor.

Fix all three by creating new entries without auto-chaining (passing
NULL as head), cloning before chaining, and checking the clone return
value.

Reported-by: syzbot+acc3b75c010446ad403f@syzkaller.appspotmail.com
Signed-off-by: zhanghaotian <zhanghaotian@uniontech.com>
---
 .../media/test-drivers/vidtv/vidtv_channel.c  | 55 +++++++++++++------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_channel.c b/drivers/media/test-drivers/vidtv/vidtv_channel.c
index 5f8c3af87..dee782d63 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_channel.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_channel.c
@@ -163,6 +163,7 @@ static struct vidtv_psi_table_eit_event
 	struct vidtv_psi_table_eit_event *curr = NULL;
 	struct vidtv_psi_table_eit_event *head = NULL;
 	struct vidtv_psi_table_eit_event *tail = NULL;
+	struct vidtv_psi_table_eit_event *new_event = NULL;
 	struct vidtv_psi_desc *desc = NULL;
 	u16 event_id;
 
@@ -179,17 +180,25 @@ static struct vidtv_psi_table_eit_event
 
 		while (curr) {
 			event_id = be16_to_cpu(curr->event_id);
-			tail = vidtv_psi_eit_event_init(tail, event_id);
-			if (!tail) {
+			new_event = vidtv_psi_eit_event_init(NULL, event_id);
+			if (!new_event) {
 				vidtv_psi_eit_event_destroy(head);
 				return NULL;
 			}
 
 			desc = vidtv_psi_desc_clone(curr->descriptor);
-			vidtv_psi_desc_assign(&tail->descriptor, desc);
+			if (!desc) {
+				vidtv_psi_eit_event_destroy(new_event);
+				vidtv_psi_eit_event_destroy(head);
+				return NULL;
+			}
+			vidtv_psi_desc_assign(&new_event->descriptor, desc);
 
 			if (!head)
-				head = tail;
+				head = new_event;
+			else
+				tail->next = new_event;
+			tail = new_event;
 
 			curr = curr->next;
 		}
@@ -209,6 +218,7 @@ static struct vidtv_psi_table_sdt_service
 	struct vidtv_psi_table_sdt_service *curr = NULL;
 	struct vidtv_psi_table_sdt_service *head = NULL;
 	struct vidtv_psi_table_sdt_service *tail = NULL;
+	struct vidtv_psi_table_sdt_service *new_service = NULL;
 
 	struct vidtv_psi_desc *desc = NULL;
 	u16 service_id;
@@ -226,20 +236,25 @@ static struct vidtv_psi_table_sdt_service
 
 		while (curr) {
 			service_id = be16_to_cpu(curr->service_id);
-			tail = vidtv_psi_sdt_service_init(tail,
+			new_service = vidtv_psi_sdt_service_init(NULL,
 							  service_id,
 							  curr->EIT_schedule,
 							  curr->EIT_present_following);
-			if (!tail)
+			if (!new_service)
 				goto free;
 
 			desc = vidtv_psi_desc_clone(curr->descriptor);
-			if (!desc)
-				goto free_tail;
-			vidtv_psi_desc_assign(&tail->descriptor, desc);
+			if (!desc) {
+				vidtv_psi_sdt_service_destroy(new_service);
+				goto free;
+			}
+			vidtv_psi_desc_assign(&new_service->descriptor, desc);
 
 			if (!head)
-				head = tail;
+				head = new_service;
+			else
+				tail->next = new_service;
+			tail = new_service;
 
 			curr = curr->next;
 		}
@@ -249,8 +264,6 @@ static struct vidtv_psi_table_sdt_service
 
 	return head;
 
-free_tail:
-	vidtv_psi_sdt_service_destroy(tail);
 free:
 	vidtv_psi_sdt_service_destroy(head);
 	return NULL;
@@ -333,12 +346,14 @@ vidtv_channel_pmt_match_sections(struct vidtv_channel *channels,
 
 			/* we got a match */
 			if (curr_id == cur_chnl->program_num) {
+				struct vidtv_psi_table_pmt_stream *prev = NULL;
+
 				s = cur_chnl->streams;
 
 				/* clone the streams for the PMT */
 				while (s) {
 					e_pid = vidtv_psi_pmt_stream_get_elem_pid(s);
-					tail = vidtv_psi_pmt_stream_init(tail,
+					tail = vidtv_psi_pmt_stream_init(NULL,
 									 s->type,
 									 e_pid);
 					if (!tail) {
@@ -346,13 +361,21 @@ vidtv_channel_pmt_match_sections(struct vidtv_channel *channels,
 						return;
 					}
 
-					if (!head)
-						head = tail;
-
 					desc = vidtv_psi_desc_clone(s->descriptor);
+					if (!desc) {
+						vidtv_psi_pmt_stream_destroy(tail);
+						vidtv_psi_pmt_stream_destroy(head);
+						return;
+					}
 					vidtv_psi_desc_assign(&tail->descriptor,
 							      desc);
 
+					if (!head)
+						head = tail;
+					if (prev)
+						prev->next = tail;
+					prev = tail;
+
 					s = s->next;
 				}
 
-- 
2.30.2