| Age | Commit message (Collapse) | Author |
|---|
|
tb_xdp_properties_request() allocates room for 45 data dwords in its
252-byte response buffer. The XDomain length field is six bits wide,
however, and a malicious peer can set it to 63. After the fixed response
fields are subtracted, the driver treats this as 48 data dwords.
Commit 322e93448d90 ("thunderbolt: Clamp XDomain response data copy to
allocation size") only bounds the copy against data_len. If data_len is
at least 48, memcpy() reads 192 bytes from the 180-byte res->data array,
causing a 12-byte heap out-of-bounds read. Commit 4db2bd2ed478
("thunderbolt: Limit XDomain response copy to actual frame size") limits
the earlier copy but does not constrain this header-derived length.
Reject response data lengths that exceed the allocated source buffer
before copying them into the assembled property block.
Fixes: d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
This reverts commit e027dba038f0008df9bc9575f5c3e803e90636c6.
Alan noticed that this causes the peers send change properties to each
other continuously.
Reported-by: Borzeszkowski, Alan <alan.borzeszkowski@intel.com>
Cc: Milo Chen <cmh79479@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
|
There is a slight race between tb_remove_work() and tb_domain_remove()
which leads to dereferencing a NULL tb->root_switch pointer inside
tb_free_unplugged_xdomains():
Thread A Thread B
tb_remove_work()
tb_domain_remove()
mutex_lock(&tb->lock)
tb_stop()
/* doesn't cancel a running callback */
cancel_delayed_work(&tcm->remove_work)
...
tb_switch_remove(tb->root_switch)
tb->root_switch = NULL
mutex_unlock(&tb->lock)
mutex_lock(&tb->lock)
...
/* without checking ->root_switch */
tb_free_unplugged_xdomains(tb->root_switch)
mutex_unlock(&tb->lock)
Commit a8937f35cf39 ("thunderbolt: Remove XDomain from the bus without
holding tb->lock") doesn't seem right to move tb_free_unplugged_xdomains()
out of the &tb->lock section and the check for tb->root_switch, in
particular. It states:
For this reason separate removing the XDomain from the topology data
structures (where we need the lock) from unregistering the device from
the bus (where remove callbacks of the drivers are being called).
tb_free_unplugged_xdomains() belongs to the former group of functions
requiring the lock. And it also calls tb_xdomain_remove() which should
only be called with &tb->lock held.
Found by Linux Verification Center (linuxtesting.org) with Svace static
analysis tool.
Fixes: a8937f35cf39 ("thunderbolt: Remove XDomain from the bus without holding tb->lock")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
|
