diff options
author | Ji Luo <ji.luo@nxp.com> | 2023-04-20 16:18:28 +0800 |
---|---|---|
committer | Ji Soo Shin <jisshin@google.com> | 2023-05-26 18:31:09 +0200 |
commit | efab9548cf430f74745ac49dbe56c9e8e13c4bd8 (patch) | |
tree | 8eba64093cb4478632ca21bc2870f259bca6a17c | |
parent | 2c29144d8fc90e33601e594b3c2b922b5fc34811 (diff) | |
download | trusty-efab9548cf430f74745ac49dbe56c9e8e13c4bd8.tar.gz |
ANDROID: trusty: fix use-after-free
rework the 'trace_trusty_ipc_read_end()' to accept 'buf_id' and
'shm_cnt' instead of the 'md' which could be invalid after free.
This fixes the KFENCE dump:
[ 4108.926665][ T254] BUG: KFENCE: use-after-free read in trace_event_raw_event_trusty_ipc_read_end+0xa8/0x11c [trusty_ipc]
[ 4108.926665][ T254]
[ 4108.939893][ T254] Use-after-free read at 0x00000000d5383753 (in kfence-#49):
[ 4108.947125][ T254] trace_event_raw_event_trusty_ipc_read_end+0xa8/0x11c [trusty_ipc]
[ 4108.955127][ T254] tipc_read_iter+0x3c4/0x434 [trusty_ipc]
[ 4108.960869][ T254] do_iter_read+0x1e4/0x300
[ 4108.965243][ T254] do_readv+0xd4/0x190
[ 4108.969180][ T254] __arm64_sys_readv+0x24/0x38
[ 4108.973810][ T254] invoke_syscall+0x5c/0x11c
[ 4108.978270][ T254] el0_svc_common+0xb8/0x104
[ 4108.982731][ T254] do_el0_svc+0x30/0xc0
[ 4108.986751][ T254] el0_svc+0x30/0xac
[ 4108.990520][ T254] el0t_64_sync_handler+0x6c/0xbc
[ 4108.995415][ T254] el0t_64_sync+0x1a0/0x1a4
[ 4108.999781][ T254]
[ 4109.001975][ T254] kfence-#49: 0x00000000d990fc1e-0x00000000baff0d36, size=96, cache=kmalloc-128
[ 4109.001975][ T254]
[ 4109.013030][ T254] allocated by task 8231 on cpu 3 at 4108.225632s:
[ 4109.019554][ T254] __kmem_cache_alloc_node+0x238/0x294
[ 4109.024879][ T254] kmalloc_trace+0x54/0x180
[ 4109.029249][ T254] vds_alloc_msg_buf+0x6c/0x120 [trusty_ipc]
[ 4109.035169][ T254] dn_handle_msg+0x58/0x1bc [trusty_ipc]
[ 4109.040736][ T254] _rxvq_cb+0x14c/0x83c [trusty_ipc]
[ 4109.045955][ T254] vring_interrupt+0xa4/0xc0
[ 4109.050412][ T254] check_all_vqs+0x64/0x88 [trusty_virtio]
[ 4109.056100][ T254] process_one_work+0x244/0x50c
[ 4109.060818][ T254] worker_thread+0x268/0x488
[ 4109.065276][ T254] kthread+0x110/0x158.513421: trusty_smc: smcnr=SC_NOP
[ 4109.069209][ T254] ret_from_fork+0x10/0x20
[ 4109.076349][ T254]
[ 4109.078543][ T254] freed by task 254 on cpu 2 at 4108.917154s:
[ 4109.084489][ T254] tipc_read_iter+0x130/0x434 [trusty_ipc]
[ 4109.090232][ T254] do_iter_read+0x1e4/0x300
[ 4109.094604][ T254] do_readv+0xd4/0x190
[ 4109.098538][ T254] __arm64_sys_readv+0x24/0x38
[ 4109.103170][ T254] invoke_syscall+0x5c/0x11c
[ 4109.107630][ T254] el0_svc_common+0xb8/0x104
[ 4109.112088][ T254] do_el0_svc+0x30/0xc0
[ 4109.116111][ T254] el0_svc+0x30/0xac
[ 4109.119873][ T254] el0t_64_sync_handler+0x6c/0xbc
[ 4109.124767][ T254] el0t_64_sync+0x1a0/0x1a4
[ 4109.129136][ T254]
[ 4109.131328][ T254] CPU: 2 PID: 254 Comm: android.hardwar Tainted: G B C OE 6.1.22-13799-g07571651e990-ab127 #1
Bug: 279270023
Test: run CtsKeystoreTestCases CTS module while enabling tracing.
Change-Id: Ide61b535bb167cce8b25ff6f6670c96c0474c885
Signed-off-by: Ji Luo <ji.luo@nxp.com>
(cherry picked from commit ec3670c83983dc14bbb23db15ff51ee40c42b537)
-rw-r--r-- | drivers/trusty/trusty-ipc-trace.h | 9 | ||||
-rw-r--r-- | drivers/trusty/trusty-ipc.c | 6 |
2 files changed, 10 insertions, 5 deletions
diff --git a/drivers/trusty/trusty-ipc-trace.h b/drivers/trusty/trusty-ipc-trace.h index 9f193e2..60d6338 100644 --- a/drivers/trusty/trusty-ipc-trace.h +++ b/drivers/trusty/trusty-ipc-trace.h @@ -176,8 +176,9 @@ TRACE_EVENT(trusty_ipc_read, TRACE_EVENT(trusty_ipc_read_end, TP_PROTO(struct tipc_chan *chan, int len_or_err, - struct tipc_msg_buf *rxbuf), - TP_ARGS(chan, len_or_err, rxbuf), + trusty_shared_mem_id_t buf_id, + size_t shm_cnt), + TP_ARGS(chan, len_or_err, buf_id, shm_cnt), TP_STRUCT__entry( __field(int, len_or_err) __field(u32, chan) @@ -189,8 +190,8 @@ TRACE_EVENT(trusty_ipc_read_end, __entry->len_or_err = len_or_err; __entry->chan = chan ? chan->local : ~0U; memcpy(__entry->srv_name, chan ? chan->srv_name : "", MAX_SRV_NAME_LEN); - __entry->buf_id = rxbuf ? rxbuf->buf_id : ~0ULL; - __entry->shm_cnt = rxbuf ? rxbuf->shm_cnt : 0; + __entry->buf_id = buf_id; + __entry->shm_cnt = shm_cnt; ), TP_printk("len_or_err=%d chan=%u srv_name=%s buf_id=0x%llx shm_cnt=%zu", __entry->len_or_err, __entry->chan, __entry->srv_name, diff --git a/drivers/trusty/trusty-ipc.c b/drivers/trusty/trusty-ipc.c index 26648c9..7ecfcf6 100644 --- a/drivers/trusty/trusty-ipc.c +++ b/drivers/trusty/trusty-ipc.c @@ -1521,6 +1521,8 @@ static ssize_t tipc_read_iter(struct kiocb *iocb, struct iov_iter *iter) { ssize_t ret; size_t len; + trusty_shared_mem_id_t buf_id = ~0ULL; + size_t shm_cnt = 0; struct tipc_msg_buf *mb = NULL; struct file *filp = iocb->ki_filp; struct tipc_dn_chan *dn = filp->private_data; @@ -1555,6 +1557,8 @@ static ssize_t tipc_read_iter(struct kiocb *iocb, struct iov_iter *iter) mb = list_first_entry(&dn->rx_msg_queue, struct tipc_msg_buf, node); + buf_id = mb->buf_id; + shm_cnt = mb->shm_cnt; len = mb_avail_data(mb); if (len > iov_iter_count(iter)) { ret = -EMSGSIZE; @@ -1571,7 +1575,7 @@ static ssize_t tipc_read_iter(struct kiocb *iocb, struct iov_iter *iter) tipc_chan_put_rxbuf(dn->chan, mb); out: - trace_trusty_ipc_read_end(dn->chan, ret, mb); + trace_trusty_ipc_read_end(dn->chan, ret, buf_id, shm_cnt); mutex_unlock(&dn->lock); return ret; } |