linux-kernel - [PATCH] thunderbolt: fix a missing-check bug

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: <1540066556-18088-1-git-send-email-wang6495@umn.edu>
Date:   Sat, 20 Oct 2018 15:15:56 -0500
From:   Wenwen Wang <wang6495@....edu>
To:     Wenwen Wang <wang6495@....edu>
Cc:     Kangjie Lu <kjlu@....edu>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] thunderbolt: fix a missing-check bug

In tb_ring_poll(), the flag of the frame, i.e.,
'ring->descriptors[ring->tail].flags', is checked to see whether the frame
is completed. If yes, the frame including the flag will be read from the
ring and returned to the caller. The problem here is that the flag is
actually in a DMA region, which is allocated through dma_alloc_coherent()
in tb_ring_alloc(). Given that the device can also access this DMA region,
it is possible that a malicious device controlled by an attacker can modify
the flag between the check and the copy. By doing so, the attacker can
bypass the check and supply uncompleted frame, which can cause undefined
behavior of the kernel and introduce potential security risk.

This patch firstly copies the flag into a local variable 'desc_flags' and
then performs the check and copy using 'desc_flags'. Through this way, the
above issue can be avoided.

Signed-off-by: Wenwen Wang <wang6495@....edu>
---
 drivers/thunderbolt/nhi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 5cd6bdf..481b1f2 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -289,6 +289,7 @@ struct ring_frame *tb_ring_poll(struct tb_ring *ring)
 {
 	struct ring_frame *frame = NULL;
 	unsigned long flags;
+	enum ring_desc_flags desc_flags;

 	spin_lock_irqsave(&ring->lock, flags);
 	if (!ring->running)
@@ -296,7 +297,8 @@ struct ring_frame *tb_ring_poll(struct tb_ring *ring)
 	if (ring_empty(ring))
 		goto unlock;

-	if (ring->descriptors[ring->tail].flags & RING_DESC_COMPLETED) {
+	desc_flags = ring->descriptors[ring->tail].flags;
+	if (desc_flags & RING_DESC_COMPLETED) {
 		frame = list_first_entry(&ring->in_flight, typeof(*frame),
 					 list);
 		list_del_init(&frame->list);
@@ -305,7 +307,7 @@ struct ring_frame *tb_ring_poll(struct tb_ring *ring)
 			frame->size = ring->descriptors[ring->tail].length;
 			frame->eof = ring->descriptors[ring->tail].eof;
 			frame->sof = ring->descriptors[ring->tail].sof;
-			frame->flags = ring->descriptors[ring->tail].flags;
+			frame->flags = desc_flags;
 		}

 		ring->tail = (ring->tail + 1) % ring->size;
-- 
2.7.4