Re: [PATCH] thunderbolt: fix a missing-check bug

From: Mika Westerberg
Date: Mon Oct 22 2018 - 04:05:33 EST


Hi,

On Sat, Oct 20, 2018 at 01:38:18PM -0500, Wenwen Wang wrote:
> 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.

Same comment here than with the previous one.