relayfs: fix out-of-bounds access in relay_file_read
authorZhang Zhengming <zhang.zhengming@h3c.com>
Wed, 19 Apr 2023 04:02:03 +0000 (12:02 +0800)
committerAndrew Morton <akpm@linux-foundation.org>
Wed, 3 May 2023 00:23:27 +0000 (17:23 -0700)
There is a crash in relay_file_read, as the var from
point to the end of last subbuf.

The oops looks something like:
pc : __arch_copy_to_user+0x180/0x310
lr : relay_file_read+0x20c/0x2c8
Call trace:
 __arch_copy_to_user+0x180/0x310
 full_proxy_read+0x68/0x98
 vfs_read+0xb0/0x1d0
 ksys_read+0x6c/0xf0
 __arm64_sys_read+0x20/0x28
 el0_svc_common.constprop.3+0x84/0x108
 do_el0_svc+0x74/0x90
 el0_svc+0x1c/0x28
 el0_sync_handler+0x88/0xb0
 el0_sync+0x148/0x180

We get the condition by analyzing the vmcore:

1). The last produced byte and last consumed byte
    both at the end of the last subbuf

2). A softirq calls function(e.g __blk_add_trace)
    to write relay buffer occurs when an program is calling
    relay_file_read_avail().

        relay_file_read
                relay_file_read_avail
                        relay_file_read_consume(buf, 0, 0);
                        //interrupted by softirq who will write subbuf
                        ....
                        return 1;
                //read_start point to the end of the last subbuf
                read_start = relay_file_read_start_pos
                //avail is equal to subsize
                avail = relay_file_read_subbuf_avail
                //from  points to an invalid memory address
                from = buf->start + read_start
                //system is crashed
                copy_to_user(buffer, from, avail)

Link: https://lkml.kernel.org/r/20230419040203.37676-1-zhang.zhengming@h3c.com
Fixes: 8d62fdebdaf9 ("relay file read: start-pos fix")
Signed-off-by: Zhang Zhengming <zhang.zhengming@h3c.com>
Reviewed-by: Zhao Lei <zhao_lei1@hoperun.com>
Reviewed-by: Zhou Kete <zhou.kete@h3c.com>
Reviewed-by: Pengcheng Yang <yangpc@wangsu.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
kernel/relay.c

index 9aa70ae53d24d13d7ba714d6659512a374d55570..a80fa01042e980324809f32a81b6e468c53be62e 100644 (file)
@@ -989,7 +989,8 @@ static size_t relay_file_read_start_pos(struct rchan_buf *buf)
        size_t subbuf_size = buf->chan->subbuf_size;
        size_t n_subbufs = buf->chan->n_subbufs;
        size_t consumed = buf->subbufs_consumed % n_subbufs;
-       size_t read_pos = consumed * subbuf_size + buf->bytes_consumed;
+       size_t read_pos = (consumed * subbuf_size + buf->bytes_consumed)
+                       % (n_subbufs * subbuf_size);
 
        read_subbuf = read_pos / subbuf_size;
        padding = buf->padding[read_subbuf];