Messages in this thread | | | From | Wenwen Wang <> | Date | Mon, 22 Oct 2018 08:02:14 -0500 | Subject | Re: [PATCH] thunderbolt: Fix a missing-check bug |
| |
On Mon, Oct 22, 2018 at 3:04 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Sat, Oct 20, 2018 at 12:55:51PM -0500, Wenwen Wang wrote: > > In tb_ctl_rx_callback(), the checksum of the received control packet is > > calculated on 'pkg->buffer' through tb_crc() and saved to 'crc32', Then, > > 'crc32' is compared with the received checksum to confirm the integrity of > > the received packet. If the checksum does not match, the packet will be > > dropped. In the following execution, 'pkg->buffer' will be copied through > > req->copy() and processed if there is an active request and the packet is > > what is expected. > > > > The problem here is that the above checking process is performed directly > > on the buffer 'pkg->buffer', which is actually a DMA region. Given that the > > DMA region can also be accessed directly by a device at any time, it is > > possible that a malicious device controlled by an attacker can race to > > modify the content in 'pkg->buffer' after the checksum checking but before > > req->copy(). By doing so, the attacker can inject malicious data, which can > > cause undefined behavior of the kernel and introduce potential security > > risk. > > > > This patch allocates a new buffer 'buf' to hold the data in 'pkg->buffer'. > > By performing the checking and copying on 'buf', rather than 'pkg->buffer', > > the above issue can be avoided. > > Here same comment applies than to the previous one - this is something > that requires the attacker to have physical access to the system and > requires him to either replace the firmware or the hardware itself with > a malicious one and in that case protection like this here does not > actually help because they can just overwrite it directly. > > BTW, just in case you send multiple patches to other subsystems as well > it is good to have $subject contain summary of the fix in a way that one > can distinguish between them. For example you sent 4 patches with all > having: > > thunderbolt: Fix a missing-check bug > > in the $subject. So for example I originally thought that you sent the > same patch several times :)
Thanks for your suggestion, Mika. That is good to distinguish between different patches :)
Wenwen
| |