linux-kernel - [PATCH v1 5/5] drivers: media: bcm2835-unicam: Correctly handle FS + FE ISR condition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241122084152.1841419-6-naush@raspberrypi.com>
Date: Fri, 22 Nov 2024 08:41:52 +0000
From: Naushir Patuck <naush@...pberrypi.com>
To: Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	Ray Jui <rjui@...adcom.com>,
	Scott Branden <sbranden@...adcom.com>
Cc: linux-media@...r.kernel.org,
	linux-rpi-kernel@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	jacopo.mondi@...asonboard.com,
	Dave Stevenson <dave.stevenson@...pberrypi.com>,
	Naushir Patuck <naush@...pberrypi.com>
Subject: [PATCH v1 5/5] drivers: media: bcm2835-unicam: Correctly handle FS + FE ISR condition

This change aligns the FS/FE interrupt handling with the Raspberry Pi
kernel downstream Unicam driver.

If we get a simultaneous FS + FE interrupt for the same frame, it cannot
be marked as completed and returned to userland as the framebuffer will
be refilled by Unicam on the next sensor frame. Additionally, the
timestamp will be set to 0 as the FS interrupt handling code will not
have run yet.

To avoid these problems, the frame is considered dropped in the FE
handler, and will be returned to userland on the subsequent sensor frame.

Signed-off-by: Naushir Patuck <naush@...pberrypi.com>
---
 .../media/platform/broadcom/bcm2835-unicam.c  | 39 +++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index f10064107d54..0d2aa25d483f 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -773,10 +773,26 @@ static irqreturn_t unicam_isr(int irq, void *dev)
 			 * as complete, as the HW will reuse that buffer.
 			 */
 			if (node->cur_frm && node->cur_frm != node->next_frm) {
+				/*
+				 * This condition checks if FE + FS for the same
+				 * frame has occurred. In such cases, we cannot
+				 * return out the frame, as no buffer handling
+				 * or timestamping has yet been done as part of
+				 * the FS handler.
+				 */
+				if (!node->cur_frm->vb.vb2_buf.timestamp) {
+					dev_dbg(unicam->v4l2_dev.dev,
+						"ISR: FE without FS, dropping frame\n");
+					continue;
+				}
+
 				unicam_process_buffer_complete(node, sequence);
+				node->cur_frm = node->next_frm;
+				node->next_frm = NULL;
 				inc_seq = true;
+			} else {
+				node->cur_frm = node->next_frm;
 			}
-			node->cur_frm = node->next_frm;
 		}
 
 		/*
@@ -812,10 +828,25 @@ static irqreturn_t unicam_isr(int irq, void *dev)
 					i);
 			/*
 			 * Set the next frame output to go to a dummy frame
-			 * if we have not managed to obtain another frame
-			 * from the queue.
+			 * if no buffer currently queued.
 			 */
-			unicam_schedule_dummy_buffer(node);
+			if (!node->next_frm ||
+			    node->next_frm == node->cur_frm) {
+				unicam_schedule_dummy_buffer(node);
+			} else if (unicam->node[i].cur_frm) {
+				/*
+				 * Repeated FS without FE. Hardware will have
+				 * swapped buffers, but the cur_frm doesn't
+				 * contain valid data. Return cur_frm to the
+				 * queue.
+				 */
+				spin_lock(&node->dma_queue_lock);
+				list_add_tail(&node->cur_frm->list,
+					      &node->dma_queue);
+				spin_unlock(&node->dma_queue_lock);
+				node->cur_frm = node->next_frm;
+				node->next_frm = NULL;
+			}
 		}
 
 		unicam_queue_event_sof(unicam);
-- 
2.34.1