From patchwork Mon May 4 13:00:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yann Droneaud X-Patchwork-Id: 6326691 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4EBE9BEEE1 for ; Mon, 4 May 2015 13:01:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 411D720382 for ; Mon, 4 May 2015 13:01:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 25389202E6 for ; Mon, 4 May 2015 13:01:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752834AbbEDNBo (ORCPT ); Mon, 4 May 2015 09:01:44 -0400 Received: from smtp2-g21.free.fr ([212.27.42.2]:31835 "EHLO smtp2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752829AbbEDNBo (ORCPT ); Mon, 4 May 2015 09:01:44 -0400 Received: from localhost.localdomain (unknown [37.162.185.86]) by smtp2-g21.free.fr (Postfix) with ESMTPS id 7C2324B0163; Mon, 4 May 2015 15:01:08 +0200 (CEST) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.14.9/8.14.8) with ESMTP id t44D1dab006843; Mon, 4 May 2015 15:01:39 +0200 Received: (from ydroneaud@localhost) by localhost.localdomain (8.14.9/8.14.9/Submit) id t44D1W0b006842; Mon, 4 May 2015 15:01:32 +0200 From: Yann Droneaud To: Doug Ledford , Roland Dreier Cc: linux-rdma@vger.kernel.org, Yann Droneaud Subject: [PATCHv1 2/6] IB/uverbs: check userspace output buffer size Date: Mon, 4 May 2015 15:00:48 +0200 Message-Id: X-Mailer: git-send-email 2.1.0 In-Reply-To: References: In-Reply-To: References: Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This patch makes uverbs functions check the length of the output buffer. This will prevent uverbs from writing past userspace provided buffer. Additionally, it will prevent an underflow while computing remaining output space. Such underflow would set outlen field in struct ib_udata in call to INIT_UDATA() to a meaningless value. For example: INIT_UDATA(&udata, buf + sizeof cmd, (unsigned long) cmd.response + sizeof resp, in_len - sizeof cmd, out_len - sizeof resp); If out_len is less than sizeof(resp), the result of subtraction is a negative value since out_len is an int per function prototype. But outlen field in struct ib_udata is unsigned, so the result is an overly large value in struct ib_udata. Such value will make further size checking useless, allowing more out of bound write in providers (eg. hw/) code. Note that checking the output size and preventing the underflow and/or overflow might break existing userspace program that incorrectly set the buffer length in a uverbs request. It's theoretically possible for a userspace driver to send a request with a wrong size that can still be processed correctly with the kernel: Let's build a request through fake 'libibverbs' and 'libvendor-rdma' userspace driver. It's a request which might be valid with current kernel but will set outlen to (size_t) -1 while it will be possible for vendor driver (eg. provider, under hw/) to write vendor fields: #include /* aka. struct ib_uverbs_cmd_hdr */ struct ibv_cmd_hdr { __u32 command; __u16 in_words; __u16 out_words; }; struct vendor_resize_cq { struct ibv_resize_cq ibv_cmd; /* vendor defined fields */ }; struct vendor_resize_cq_resp resp { struct ibv_resize_cq_resp ibv_resp; /* vendor defined fields */ }; struct ibv_cq *vendor_resize_cq_broken_outlen(...) { struct vendor_resize_cq cmd; struct vendor_resize_cq_resp resp; size_t resp_size; ... /* slightly less than struct ib_uverbs_resize_cq_resp */ resp_size = sizeof(struct ibv_create_cq_resp); resp_size--; ... IBV_INIT_CMD_RESP(&cmd, sizeof(cmd), RESIZE_CQ, &resp, resp_size); ... write(context->cmd_fd, &cmd, sizeof(cmd)); ... } This example shows how a request can be created to trigger un-handled underflow while allowing provider (eg. hw/) to process the request. The provided patch will make it impossible to do it on patched kernels. Applications using libibverbs are not going to be affected by this patch, but unknown broken applications using directly kernel uverbs API may fail. Link: http://marc.info/?i=1387493822.11925.217.camel@localhost.localdomain Link: http://marc.info/?i=cover.1430743694.git.ydroneaud@opteya.com Signed-off-by: Yann Droneaud --- drivers/infiniband/core/uverbs_cmd.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index 6d26bfab1bc6..a3a3b6eafd1d 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -1486,6 +1486,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1554,6 +1557,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1633,6 +1639,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1982,6 +1991,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2198,6 +2210,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2261,6 +2276,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2505,6 +2523,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2557,6 +2578,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -3356,6 +3380,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file, if (in_len < sizeof cmd) return -EINVAL; + if (out_len < sizeof resp) + return -ENOSPC; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT;