Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Oct 2018 12:13:19 +0300
From:   Mika Westerberg <mika.westerberg@linux.intel.com>
To:     Wenwen Wang <wang6495@umn.edu>
Cc:     Kangjie Lu <kjlu@umn.edu>,
        Andreas Noever <andreas.noever@gmail.com>,
        Michael Jamet <michael.jamet@intel.com>,
        Yehezkel Bernat <YehezkelShB@gmail.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] thunderbolt: Fix a missing-check bug
Message-ID: <20181018091319.GT2302@lahna.fi.intel.com>
References: <1539784829-1159-1-git-send-email-wang6495@umn.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1539784829-1159-1-git-send-email-wang6495@umn.edu>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Wenwen,

On Wed, Oct 17, 2018 at 09:00:29AM -0500, Wenwen Wang wrote:
> In tb_cfg_copy(), the header of the received control package, which is in
> the buffer 'pkg->buffer', is firstly parsed through parse_header() to make
> sure the header is in the expected format. In parse_header(), the header is
> actually checked by invoking check_header(), which checks the 'unknown'
> field of the header and the response route acquired through
> 'tb_cfg_get_route(header)'. If there is no error in this checking process,
> the package, including the header, is then copied to 'req->response' in
> tb_cfg_copy() through memcpy() and the parsing result is saved to
> 'req->result'.
> 
> The problem here is that the whole parsing and checking process is
> conducted directly on the buffer 'pkg->buffer', which is actually a DMA
> region and allocated through dma_pool_alloc() in tb_ctl_pkg_alloc(). Given
> that the DMA region can also be accessed directly by a device at any time,
> it is possible that a malicious device can race to modify the package data
> after the parsing and checking process but before memcpy() is invoked in
> tb_cfg_copy(). Through this way, the attacker can bypass the parsing and
> checking process and inject malicious data. This can potentially cause
> undefined behavior of the kernel and introduce unexpected security risk.

Here the device doing DMA is the Thunderbolt host controller which is
soldered on the motherboard (not anything connected via the TBT ports).
In addition the buffers we are dealing here are already marked ready by
the host controller hardware so it is not expected to touch them anymore
(if it did, then it would be a quite nasty bug).

What kind of use-case you had in mind that could possibly inject
malicious data to these buffers?