linux-kernel - RE: [PATCH] iw_cxgb4: 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] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 20 Oct 2018 18:14:12 -0500
From:   "Steve Wise" <swise@...ngridcomputing.com>
To:     "'Wenwen Wang'" <wang6495@....edu>
Cc:     "'Kangjie Lu'" <kjlu@....edu>, "'Steve Wise'" <swise@...lsio.com>,
        "'Doug Ledford'" <dledford@...hat.com>,
        "'Jason Gunthorpe'" <jgg@...pe.ca>,
        "'open list:CXGB4 IWARP RNIC DRIVER \(IW_CXGB4\)'" 
        <linux-rdma@...r.kernel.org>,
        "'open list'" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] iw_cxgb4: fix a missing-check bug

Hey Wenwen,

> Subject: [PATCH] iw_cxgb4: fix a missing-check bug
> 
> In c4iw_flush_hw_cq, the next CQE is acquired through t4_next_hw_cqe(). In
> t4_next_hw_cqe(), the CQE, i.e., 'cq->queue[cq->cidx]', is checked to see
> whether it is valid through t4_valid_cqe(). If it is valid, the address of
> the CQE is then saved to 'hw_cqe'. Later on, the CQE is copied to the
local
> memory in create_read_req_cqe(). The problem here is that the CQE is
> actually in a DMA region allocated by dma_alloc_coherent() in create_cq().
> Given that the device also has the permission to access the DMA region, a
> malicious device controlled by an attacker can modify the CQE in the DMA
> region after the check in t4_next_hw_cqe() but before the copy in
> create_read_req_cqe(). By doing so, the attacker can supply invalid CQE,
> which can cause undefined behavior of the kernel and introduce potential
> security risks.
> 

If the dma device is malicious, couldn't it just dma some incorrect CQE but
still valid in the first place?  I don't think this patch actually solves
the issue, and it forces a copy of a 64B CQE in a critical data io path.  

So I must NACK this. 

Thanks,

Steve.