From patchwork Sun Jul 20 19:51:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yann Droneaud X-Patchwork-Id: 4592131 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.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C6F4BC0514 for ; Sun, 20 Jul 2014 19:53:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6B64F2010B for ; Sun, 20 Jul 2014 19:53:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 10B52200E8 for ; Sun, 20 Jul 2014 19:53:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753273AbaGTTxm (ORCPT ); Sun, 20 Jul 2014 15:53:42 -0400 Received: from smtp6-g21.free.fr ([212.27.42.6]:19761 "EHLO smtp6-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753008AbaGTTxl (ORCPT ); Sun, 20 Jul 2014 15:53:41 -0400 Received: from localhost.localdomain (unknown [IPv6:2a01:e35:2e9f:6ac0:446c:9ad5:44d4:79a1]) by smtp6-g21.free.fr (Postfix) with ESMTP id 70D4882259; Sun, 20 Jul 2014 21:53:36 +0200 (CEST) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.14.8/8.14.7) with ESMTP id s6KJqUtF006091; Sun, 20 Jul 2014 21:52:35 +0200 Received: (from ydroneaud@localhost) by localhost.localdomain (8.14.8/8.14.8/Submit) id s6KJqU5S006090; Sun, 20 Jul 2014 21:52:30 +0200 From: Yann Droneaud To: Roland Dreier , Sean Hefty , Hal Rosenstock Cc: linux-rdma@vger.kernel.org, Yann Droneaud Subject: [PATCH 1/4] IB/uverbs: check userspace input buffer size Date: Sun, 20 Jul 2014 21:51:09 +0200 Message-Id: <1e64a875d1dacc592996133ef31defb32d3b5d2a.1405884453.git.ydroneaud@opteya.com> X-Mailer: git-send-email 1.9.3 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=-5.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY, URIBL_RHS_DOB autolearn=ham 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 input buffer before reading the command content. This will detect truncated command and will prevent uverbs from reading past userspace provided buffer. Additionally, it will also prevent underflow while computing remaining input space. Such underflow would set inlen 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 in_len is less than sizeof(cmd), the result of subtraction is a negative value since in_len is an int per function prototype. But inlen 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 read in providers (eg. hw/) code. Note that checking the input 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 inlen to (size_t) -1 while it will be possible for vendor driver (eg. provider, under hw/) to read vendor fields: #include /* aka. struct ib_uverbs_cmd_hdr */ struct ibv_cmd_hdr { __u32 command; __u16 in_words; __u16 out_words; }; struct vendor_create_cq { struct ibv_create_cq ibv_cmd; /* vendor defined fields */ }; struct vendor_create_cq_resp resp { struct ibv_create_cq_resp ibv_resp; /* vendor defined fields */ }; struct ibv_cq *vendor_create_cq_broken_inlen(...) { struct vendor_create_cq cmd; struct vendor_create_cq_resp resp; size_t cmd_size; /* the command header size must be subtracted here to ensure (inlen - sizeof(struct ib_uverbs_create_cq) is equal to 0 in ib_uverbs_create_cq(), as the header size is not subtracted in ib_uverbs_write() before invoking ib_uverbs_create_cq() */ assert(sizeof(struct ibv_create_cq) >= sizeof(struct ibv_cmd_hdr)); cmd_size = sizeof(struct ibv_create_cq) - sizeof(struct ibv_cmd_hdr); /* instead of 0, make (inlen - sizeof(cmd)) underflow */ cmd_size--; /* For the command to be valid in ib_uverbs_write(), it size must be at least equal to sizeof(struct ib_uverbs_cmd_hdr). */ assert(cmd_size >= sizeof(struct ibv_cmd_hdr)); ... IBV_INIT_CMD_RESP(&cmd, cmd_size, CREATE_CQ, &resp, sizeof(resp)); ... write(context->cmd_fd, &cmd, cmd_size); ... } 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, but might make unknown broken applications fail. Link: http://marc.info/?i=cover.1405884453.git.ydroneaud@opteya.com Link: http://marc.info/?i=1387493822.11925.217.camel@localhost.localdomain Signed-off-by: Yann Droneaud --- drivers/infiniband/core/uverbs_cmd.c | 102 +++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index ea6203ee7bcc..353cbd5229bb 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -292,6 +292,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file, struct file *filp; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -387,6 +390,9 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file, struct ib_device_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -456,6 +462,9 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file, struct ib_port_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -508,6 +517,9 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file, struct ib_pd *pd; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -579,6 +591,9 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file, struct ib_uobject *uobj; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -705,6 +720,9 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file, int ret = 0; int new_xrcd = 0; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -840,6 +858,9 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file, int live; int ret = 0; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -917,6 +938,9 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file, struct ib_mr *mr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -1011,6 +1035,9 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file, struct ib_uobject *uobj; int ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1051,6 +1078,9 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file, struct ib_mw *mw; int ret; + if (in_len < sizeof(cmd)) + return -EINVAL; + if (out_len < sizeof(resp)) return -ENOSPC; @@ -1131,6 +1161,9 @@ ssize_t ib_uverbs_dealloc_mw(struct ib_uverbs_file *file, struct ib_uobject *uobj; int ret = -EINVAL; + if (in_len < sizeof(cmd)) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof(cmd))) return -EFAULT; @@ -1169,6 +1202,9 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file, struct file *filp; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -1209,6 +1245,9 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file, struct ib_cq *cq; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -1308,6 +1347,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file, struct ib_cq *cq; int ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1373,6 +1415,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file, struct ib_wc wc; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1419,6 +1464,9 @@ ssize_t ib_uverbs_req_notify_cq(struct ib_uverbs_file *file, struct ib_uverbs_req_notify_cq cmd; struct ib_cq *cq; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1446,6 +1494,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file, struct ib_uverbs_event_file *ev_file; int ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1504,6 +1555,9 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file, struct ib_qp_init_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -1694,6 +1748,9 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file, struct ib_qp_open_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -1786,6 +1843,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file, struct ib_qp_init_attr *init_attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1899,6 +1959,9 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file, struct ib_qp_attr *attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -1995,6 +2058,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file, struct ib_uqp_object *obj; int ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2055,6 +2121,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file, int is_ud; ssize_t ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2296,6 +2365,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file, struct ib_qp *qp; ssize_t ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2345,6 +2417,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file, struct ib_srq *srq; ssize_t ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2396,6 +2471,9 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file, struct ib_ah_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -2482,6 +2560,9 @@ ssize_t ib_uverbs_destroy_ah(struct ib_uverbs_file *file, struct ib_uobject *uobj; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2520,6 +2601,9 @@ ssize_t ib_uverbs_attach_mcast(struct ib_uverbs_file *file, struct ib_uverbs_mcast_entry *mcast; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2567,6 +2651,9 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file, struct ib_uverbs_mcast_entry *mcast; int ret = -EINVAL; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -2982,6 +3069,9 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file, struct ib_udata udata; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -3015,6 +3105,9 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file, struct ib_udata udata; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -3042,6 +3135,9 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file, struct ib_srq_attr attr; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT; @@ -3072,6 +3168,9 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file, struct ib_srq *srq; int ret; + if (in_len < sizeof cmd) + return -EINVAL; + if (out_len < sizeof resp) return -ENOSPC; @@ -3115,6 +3214,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file, struct ib_usrq_object *us; enum ib_srq_type srq_type; + if (in_len < sizeof cmd) + return -EINVAL; + if (copy_from_user(&cmd, buf, sizeof cmd)) return -EFAULT;