Messages in this thread Patch in this message | | | From | Wenwen Wang <> | Subject | [PATCH] thunderbolt: Fix a missing-check bug | Date | Sat, 20 Oct 2018 12:55:51 -0500 |
| |
In tb_ctl_rx_callback(), the checksum of the received control packet is calculated on 'pkg->buffer' through tb_crc() and saved to 'crc32', Then, 'crc32' is compared with the received checksum to confirm the integrity of the received packet. If the checksum does not match, the packet will be dropped. In the following execution, 'pkg->buffer' will be copied through req->copy() and processed if there is an active request and the packet is what is expected.
The problem here is that the above checking process is performed directly on the buffer 'pkg->buffer', which is actually a DMA region. Given that the DMA region can also be accessed directly by a device at any time, it is possible that a malicious device controlled by an attacker can race to modify the content in 'pkg->buffer' after the checksum checking but before req->copy(). By doing so, the attacker can inject malicious data, which can cause undefined behavior of the kernel and introduce potential security risk.
This patch allocates a new buffer 'buf' to hold the data in 'pkg->buffer'. By performing the checking and copying on 'buf', rather than 'pkg->buffer', the above issue can be avoided.
Signed-off-by: Wenwen Wang <wang6495@umn.edu> --- drivers/thunderbolt/ctl.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c index 37a7f4c..9e40572 100644 --- a/drivers/thunderbolt/ctl.c +++ b/drivers/thunderbolt/ctl.c @@ -409,6 +409,8 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame, struct ctl_pkg *pkg = container_of(frame, typeof(*pkg), frame); struct tb_cfg_request *req; __be32 crc32; + void *pkg_buf = pkg->buffer; + void *buf = NULL; if (canceled) return; /* @@ -422,6 +424,13 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame, goto rx; } + buf = kzalloc(frame->size, GFP_KERNEL); + if (!buf) + goto rx; + + memcpy(buf, pkg->buffer, frame->size); + pkg->buffer = buf; + frame->size -= 4; /* remove checksum */ crc32 = tb_crc(pkg->buffer, frame->size); be32_to_cpu_array(pkg->buffer, pkg->buffer, frame->size / 4); @@ -476,6 +485,10 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame, } rx: + if (buf) { + pkg->buffer = pkg_buf; + kfree(buf); + } tb_ctl_rx_submit(pkg); } -- 2.7.4
| |