v9fs.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: JP Kobryn <inwardvessel@gmail.com>
To: ericvh@kernel.org, lucho@ionkov.net, asmadeus@codewreck.org,
	linux_oss@crudebyte.com, rostedt@goodmis.org,
	mhiramat@kernel.org, mathieu.desnoyers@efficios.com
Cc: v9fs@lists.linux.dev, linux-trace-kernel@vger.kernel.org,
	bpf@vger.kernel.org, kernel-team@meta.com
Subject: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
Date: Fri,  1 Dec 2023 19:04:10 -0800	[thread overview]
Message-ID: <20231202030410.61047-1-inwardvessel@gmail.com> (raw)

An out of bounds read can occur within the tracepoint 9p_protocol_dump().
In the fast assign, there is a memcpy that uses a constant size of 32
(macro definition as P9_PROTO_DUMP_SZ). When the copy is invoked, the
source buffer is not guaranteed match this size. It was found that in some
cases the source buffer size is less than 32, resulting in a read that
overruns.

The size of the source buffer seems to be known at the time of the
tracepoint being invoked. The allocations happen within p9_fcall_init(),
where the capacity field is set to the allocated size of the payload
buffer. This patch tries to fix the overrun by using the minimum of that
field (size of source buffer) and the size of destination buffer when
performing the copy.

A repro can be performed by different ways. The simplest might just be
mounting a shared filesystem (between host and guest vm) using the plan
9 protocol while the tracepoint is enabled.

mount -t 9p -o trans=virtio <mount_tag> <mount_path>

The bpftrace program below can be used to show the out of bounds read.
Note that a recent version of bpftrace is needed for the raw tracepoint
support. The script was tested using v0.19.0.

/* from include/net/9p/9p.h */
struct p9_fcall {
    u32 size;
    u8 id;
    u16 tag;
    size_t offset;
    size_t capacity;
    struct kmem_cache *cache;
    u8 *sdata;
    bool zc;
};

tracepoint:9p:9p_protocol_dump
{
    /* out of bounds read can happen when this tracepoint is enabled */
}

rawtracepoint:9p_protocol_dump
{
    $pdu = (struct p9_fcall *)arg1;
    $dump_sz = (uint64)32;

    if ($dump_sz > $pdu->capacity) {
        printf("reading %zu bytes from src buffer of %zu bytes\n",
            $dump_sz, $pdu->capacity);
    }
}

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 include/trace/events/9p.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/9p.h b/include/trace/events/9p.h
index 4dfa6d7f83ba..8690a7086252 100644
--- a/include/trace/events/9p.h
+++ b/include/trace/events/9p.h
@@ -185,7 +185,8 @@ TRACE_EVENT(9p_protocol_dump,
 		    __entry->clnt   =  clnt;
 		    __entry->type   =  pdu->id;
 		    __entry->tag    =  pdu->tag;
-		    memcpy(__entry->line, pdu->sdata, P9_PROTO_DUMP_SZ);
+		    memcpy(__entry->line, pdu->sdata,
+				min(pdu->capacity, P9_PROTO_DUMP_SZ));
 		    ),
 	    TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
 		      (unsigned long)__entry->clnt, show_9p_op(__entry->type),
-- 
2.43.0


             reply	other threads:[~2023-12-02  3:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-02  3:04 JP Kobryn [this message]
2023-12-02  4:35 ` [PATCH] 9p: prevent read overrun in protocol dump tracepoint asmadeus
2023-12-02  7:19   ` asmadeus
2023-12-02 13:05   ` Christian Schoenebeck
2023-12-03  1:14     ` Steven Rostedt
2023-12-03  1:33       ` Dominique Martinet
2023-12-03  4:15         ` Steven Rostedt
2023-12-03  5:32           ` Dominique Martinet
2023-12-04 16:20             ` JP Kobryn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231202030410.61047-1-inwardvessel@gmail.com \
    --to=inwardvessel@gmail.com \
    --cc=asmadeus@codewreck.org \
    --cc=bpf@vger.kernel.org \
    --cc=ericvh@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=v9fs@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).