linux-kernel - [PATCH] videobuf2/vb2_buffer_done: Changing the position of spinlock to protect only the required code

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-next>] [day] [month] [year] [list]

Message-Id: <20180727082146epcas5p10374c04f0767dbbe409c8171c49d7c9a~FLBKL7utW2249922499epcas5p1c@epcas5p1.samsung.com>
Date:   Fri, 27 Jul 2018 13:51:36 +0530
From:   Satendra Singh Thakur <satendra.t@...sung.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Satendra Singh Thakur <satendra.t@...sung.com>,
        Al Viro <viro@...iv.linux.org.uk>, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     vineet.j@...sung.com, hemanshu.s@...sung.com, sst2005@...il.com
Subject: [PATCH] videobuf2/vb2_buffer_done: Changing the position of
 spinlock to protect only the required code

1.Currently, in the func vb2_buffer_done, spinlock protects
following code
vb->state = VB2_BUF_STATE_QUEUED;
list_add_tail(&vb->done_entry, &q->done_list);
spin_unlock_irqrestore(&q->done_lock, flags);
vb->state = state;
atomic_dec(&q->owned_by_drv_count);
2.The spinlock is mainly needed to protect list related ops and
vb->state = STATE_ERROR or STATE_DONE as in other funcs
vb2_discard_done
__vb2_get_done_vb
vb2_core_poll.
3. Therefore, spinlock is mainly needed for
   list_add, list_del, list_first_entry ops
   and state = STATE_DONE and STATE_ERROR to protect
   done_list queue.
3. Hence, state = STATE_QUEUED doesn't need spinlock protection.
4. Also atomic_dec dones't require the same as its already atomic.

Signed-off-by: Satendra Singh Thakur <satendra.t@...sung.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec73..968b403 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -923,17 +923,17 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 			call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 	}
 
-	spin_lock_irqsave(&q->done_lock, flags);
 	if (state == VB2_BUF_STATE_QUEUED ||
 	    state == VB2_BUF_STATE_REQUEUEING) {
 		vb->state = VB2_BUF_STATE_QUEUED;
 	} else {
 		/* Add the buffer to the done buffers list */
+		spin_lock_irqsave(&q->done_lock, flags);
 		list_add_tail(&vb->done_entry, &q->done_list);
 		vb->state = state;
+		spin_unlock_irqrestore(&q->done_lock, flags);
 	}
 	atomic_dec(&q->owned_by_drv_count);
-	spin_unlock_irqrestore(&q->done_lock, flags);
 
 	trace_vb2_buf_done(q, vb);
 
-- 
2.7.4