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

[Wenwen Wang]

On Thu, Oct 18, 2018 at 4:13 AM Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
>
> 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?

Hi Mika,

Thanks for your response. The current version of the code assumes that
the Thunderbolt controller behaves as expected, e.g., the host
controller should not touch the data after it is marked ready.
However, it is not impossible that the controller is exploited by an
attacker through a security vulnerability, even though it is soldered
on the motherboard. In that case, the controller may behave in an
unexpected way and this bug will offer more opportunities for the
attacker.

Wenwen