* [Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk
@ 2019-02-12 11:23 Yuval Shaia
2019-02-12 11:42 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 2+ messages in thread
From: Yuval Shaia @ 2019-02-12 11:23 UTC (permalink / raw)
To: yuval.shaia, marcel.apfelbaum, qemu-devel, sam.j.smith,
richard.b.johnson
The function get_fd extract context from the received MAD message and
uses it as a key to fetch the destination fd from the mapping table.
A context can be dgid in case of CM request message or comm_id in case
of CM SIDR response message.
When MAD message with a smaller size as expected for the message type
received we are hitting out-of-bounds where we are looking for the
context out of message boundaries.
Fix it by validating the message size.
Reported-by Sam Smith <sam.j.smith@oracle.com>
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
contrib/rdmacm-mux/main.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
index ae88c77a1e..21cc804367 100644
--- a/contrib/rdmacm-mux/main.c
+++ b/contrib/rdmacm-mux/main.c
@@ -300,7 +300,7 @@ static void hash_tbl_remove_fd_ifid_pair(int fd)
pthread_rwlock_unlock(&server.lock);
}
-static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
+static int get_fd(const char *mad, int umad_len, int *fd, __be64 *gid_ifid)
{
struct umad_hdr *hdr = (struct umad_hdr *)mad;
char *data = (char *)hdr + sizeof(*hdr);
@@ -308,13 +308,35 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
uint16_t attr_id = be16toh(hdr->attr_id);
int rc = 0;
+ if (umad_len <= sizeof(*hdr)) {
+ rc = -EINVAL;
+ syslog(LOG_DEBUG, "Ignoring MAD packets with header only\n");
+ goto out;
+ }
+
switch (attr_id) {
case UMAD_CM_ATTR_REQ:
+ if (unlikely(umad_len < sizeof(*hdr) + CM_REQ_DGID_POS +
+ sizeof(*gid_ifid))) {
+ rc = -EINVAL;
+ syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+ attr_id);
+ goto out;
+ }
memcpy(gid_ifid, data + CM_REQ_DGID_POS, sizeof(*gid_ifid));
rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
break;
case UMAD_CM_ATTR_SIDR_REQ:
+ if (unlikely(umad_len < sizeof(*hdr) + CM_SIDR_REQ_DGID_POS +
+ sizeof(*gid_ifid))) {
+ rc = -EINVAL;
+ syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+ attr_id);
+ goto out;
+ }
memcpy(gid_ifid, data + CM_SIDR_REQ_DGID_POS, sizeof(*gid_ifid));
rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
break;
@@ -331,6 +353,13 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
data += sizeof(comm_id);
/* Fall through */
case UMAD_CM_ATTR_SIDR_REP:
+ if (unlikely(umad_len < sizeof(*hdr) + sizeof(comm_id))) {
+ rc = -EINVAL;
+ syslog(LOG_WARNING,
+ "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+ attr_id);
+ goto out;
+ }
memcpy(&comm_id, data, sizeof(comm_id));
if (comm_id) {
rc = hash_tbl_search_fd_by_comm_id(comm_id, fd, gid_ifid);
@@ -344,6 +373,7 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
syslog(LOG_DEBUG, "mad_to_vm: %d 0x%x 0x%x\n", *fd, attr_id, comm_id);
+out:
return rc;
}
@@ -372,7 +402,8 @@ static void *umad_recv_thread_func(void *args)
} while (rc && server.run);
if (server.run) {
- rc = get_fd(msg.umad.mad, &fd, &msg.hdr.sgid.global.interface_id);
+ rc = get_fd(msg.umad.mad, msg.umad_len, &fd,
+ &msg.hdr.sgid.global.interface_id);
if (rc) {
continue;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk
2019-02-12 11:23 [Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk Yuval Shaia
@ 2019-02-12 11:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 2+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-02-12 11:42 UTC (permalink / raw)
To: Yuval Shaia, marcel.apfelbaum, qemu-devel, sam.j.smith,
richard.b.johnson
Cc: qemu-stable
On 2/12/19 12:23 PM, Yuval Shaia wrote:
> The function get_fd extract context from the received MAD message and
> uses it as a key to fetch the destination fd from the mapping table.
> A context can be dgid in case of CM request message or comm_id in case
> of CM SIDR response message.
>
> When MAD message with a smaller size as expected for the message type
> received we are hitting out-of-bounds where we are looking for the
> context out of message boundaries.
>
> Fix it by validating the message size.
>
Cc: qemu-stable@nongnu.org
> Reported-by Sam Smith <sam.j.smith@oracle.com>
> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> contrib/rdmacm-mux/main.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
> index ae88c77a1e..21cc804367 100644
> --- a/contrib/rdmacm-mux/main.c
> +++ b/contrib/rdmacm-mux/main.c
> @@ -300,7 +300,7 @@ static void hash_tbl_remove_fd_ifid_pair(int fd)
> pthread_rwlock_unlock(&server.lock);
> }
>
> -static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
> +static int get_fd(const char *mad, int umad_len, int *fd, __be64 *gid_ifid)
> {
> struct umad_hdr *hdr = (struct umad_hdr *)mad;
> char *data = (char *)hdr + sizeof(*hdr);
> @@ -308,13 +308,35 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
> uint16_t attr_id = be16toh(hdr->attr_id);
> int rc = 0;
>
> + if (umad_len <= sizeof(*hdr)) {
> + rc = -EINVAL;
> + syslog(LOG_DEBUG, "Ignoring MAD packets with header only\n");
> + goto out;
> + }
> +
> switch (attr_id) {
> case UMAD_CM_ATTR_REQ:
> + if (unlikely(umad_len < sizeof(*hdr) + CM_REQ_DGID_POS +
> + sizeof(*gid_ifid))) {
> + rc = -EINVAL;
> + syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
> + attr_id);
> + goto out;
> + }
> memcpy(gid_ifid, data + CM_REQ_DGID_POS, sizeof(*gid_ifid));
> rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
> break;
>
> case UMAD_CM_ATTR_SIDR_REQ:
> + if (unlikely(umad_len < sizeof(*hdr) + CM_SIDR_REQ_DGID_POS +
> + sizeof(*gid_ifid))) {
> + rc = -EINVAL;
> + syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
> + attr_id);
> + goto out;
> + }
> memcpy(gid_ifid, data + CM_SIDR_REQ_DGID_POS, sizeof(*gid_ifid));
> rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
> break;
> @@ -331,6 +353,13 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
> data += sizeof(comm_id);
> /* Fall through */
> case UMAD_CM_ATTR_SIDR_REP:
> + if (unlikely(umad_len < sizeof(*hdr) + sizeof(comm_id))) {
> + rc = -EINVAL;
> + syslog(LOG_WARNING,
> + "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
> + attr_id);
> + goto out;
> + }
> memcpy(&comm_id, data, sizeof(comm_id));
> if (comm_id) {
> rc = hash_tbl_search_fd_by_comm_id(comm_id, fd, gid_ifid);
> @@ -344,6 +373,7 @@ static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
>
> syslog(LOG_DEBUG, "mad_to_vm: %d 0x%x 0x%x\n", *fd, attr_id, comm_id);
>
> +out:
> return rc;
> }
>
> @@ -372,7 +402,8 @@ static void *umad_recv_thread_func(void *args)
> } while (rc && server.run);
>
> if (server.run) {
> - rc = get_fd(msg.umad.mad, &fd, &msg.hdr.sgid.global.interface_id);
> + rc = get_fd(msg.umad.mad, msg.umad_len, &fd,
> + &msg.hdr.sgid.global.interface_id);
> if (rc) {
> continue;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-02-12 11:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-12 11:23 [Qemu-devel] [PATCH] contrib/rdmacm-mux: Fix out-of-bounds risk Yuval Shaia
2019-02-12 11:42 ` Philippe Mathieu-Daudé
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 NNTP newsgroup(s).