linux-kernel - Re: [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-prev] [day] [month] [year] [list]

Message-ID: <20181022080526.GL2302@lahna.fi.intel.com>
Date:   Mon, 22 Oct 2018 11:05:26 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Wenwen Wang <wang6495@....edu>
Cc:     Kangjie Lu <kjlu@....edu>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] thunderbolt: fix a missing-check bug

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.