[PATCH] thunderbolt: fix a missing-check bug
From: Wenwen Wang
Date: Sat Oct 20 2018 - 14:38:31 EST
In icm_copy(), the packet id 'hdr->packet_id' is firstly compared against
'req->npackets'. If it is less than 'req->npackets', the received packet.
i.e., 'pkg->buffer', is then copied to 'req->response + offset' through
memcpy(). It is worth noting that 'offset' is also calculated based on
'hdr->packet_id'. The problem here is that both the check and the
calculation are conducted directly on 'pkg->buffer', which is actually a
DMA memory region. Given that a device can also access the DMA region at
any time, it is possible that a malicious device controlled by an attacker
can modify the packet id after the check. By doing so, the attacker can
supply comprised value into 'offset' and thus cause unexpected errors.
This patch firstly copies the header of the packet and performs the check
and the calculation on the copied version to fix the above issue. This
patch also rewrites the header in 'req->response + offset' using the
copied header to avoid a potential inconsistency issue.
Signed-off-by: Wenwen Wang <wang6495@xxxxxxx>
---
drivers/thunderbolt/icm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 28fc4ce..3beec4b 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -186,15 +186,18 @@ static bool icm_match(const struct tb_cfg_request *req,
static bool icm_copy(struct tb_cfg_request *req, const struct ctl_pkg *pkg)
{
- const struct icm_pkg_header *hdr = pkg->buffer;
+ struct icm_pkg_header hdr;
- if (hdr->packet_id < req->npackets) {
- size_t offset = hdr->packet_id * req->response_size;
+ memcpy(&hdr, pkg->buffer, sizeof(hdr));
+
+ if (hdr.packet_id < req->npackets) {
+ size_t offset = hdr.packet_id * req->response_size;
memcpy(req->response + offset, pkg->buffer, req->response_size);
+ (struct icm_pkg_header *)(req->response + offset) = hdr;
}
- return hdr->packet_id == hdr->total_packets - 1;
+ return hdr.packet_id == hdr.total_packets - 1;
}
static int icm_request(struct tb *tb, const void *request, size_t request_size,
--
2.7.4