From patchwork Fri Jul 6 20:18:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 940709 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="Q9ym/AdB"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41MmJV071cz9s2R for ; Sat, 7 Jul 2018 06:18:33 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754135AbeGFUSV (ORCPT ); Fri, 6 Jul 2018 16:18:21 -0400 Received: from mail-yb0-f201.google.com ([209.85.213.201]:49429 "EHLO mail-yb0-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754036AbeGFUST (ORCPT ); Fri, 6 Jul 2018 16:18:19 -0400 Received: by mail-yb0-f201.google.com with SMTP id t10-v6so11969750ybg.16 for ; Fri, 06 Jul 2018 13:18:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:date:message-id:subject:from:to:cc; bh=9lZE8mspbom25+QAi6lF959ttyUUF0aSqxdOEzxHOCs=; b=Q9ym/AdBF7KK4jjkprOecQUv4fRUKE3YGL2xjXKzoBofXU8cDgdQNn7vQB34wI2vzP jcdSLDkwlSEQiUbwNdyfGMQRf/llpgvqHNTC4vx8roeVuzK1gnlEIolwW7v06HUGof2b 8aI0Q20jX/M8TY2EDmL1+wBIr5IcDrbsiU9HC8Hb3axa7+lRunhxW0wLpazOTOM8sG51 vUY5hS4a8AiNrm1rYOWKhk0U7jI+XmhMgdr50XeFjI6Q4Gcsrb4gQ8h5u//UHuxdrxXS VbyC3oAtyyUBHNo4kXQCRw6IPGv8mQ/F7RAjug3qOgEWJDteqlPrRJDeBChvwBtGv4QF Jzlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:date:message-id:subject:from:to:cc; bh=9lZE8mspbom25+QAi6lF959ttyUUF0aSqxdOEzxHOCs=; b=Ndux27SuKdq1Jw9BN2IzBlZO2Yo+zOM5RERXi9uvnHLHS7s8nYVPdW+a0K3GLpebaw z6aPZ7vrpShCqQi12/bI/2q1kTtMdHJXy1xU2tj4ukcTHb7UArmFFm1i4TfPVibseDo+ rSBYYMbKfx66YJfVbzKfiwMZPnjDZciG/UdDW9JOUKUaIeDon1Fjmx+mlSxB/jh8N8CG X+UCItJhtr+uqKZU45yysUgSjwUFXm4CcrmRkYr3qPcbfO9gcTOt23CdXYjFQp/zBdDh fH/PT/2YGiJMDs+VhDPUQG0zP4JBpkDwlv9H5HwSdd/FqLZhz71kifSvxYw/JKyMfcBf ER0g== X-Gm-Message-State: APt69E1sX78d6expElQvGdMcCy1A6mzlUHKvfnIQGfzZNe15I4D5SJNL J/vZCRN5+MUU888l9WaPM6vmGzMwrg== X-Google-Smtp-Source: AAOMgpf+5BXvSreX+p9CBD0rzu8RMWCI3TJBlE6kbOXUq1fOAb48oCzOuHatct3HH1dTyT/BVfIseKCkMQ== MIME-Version: 1.0 X-Received: by 2002:a25:8686:: with SMTP id z6-v6mr3521354ybk.37.1530908298580; Fri, 06 Jul 2018 13:18:18 -0700 (PDT) Date: Fri, 6 Jul 2018 22:18:09 +0200 Message-Id: <20180706201809.105152-1-jannh@google.com> X-Mailer: git-send-email 2.18.0.203.gfac676dfb9-goog Subject: [PATCH net v2] net/mlx5: fix uaccess beyond "count" in debugfs read/write handlers From: Jann Horn To: Saeed Mahameed , Leon Romanovsky , "David S. Miller" , jannh@google.com Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org In general, accessing userspace memory beyond the length of the supplied buffer in VFS read/write handlers can lead to both kernel memory corruption (via kernel_read()/kernel_write(), which can e.g. be triggered via sys_splice()) and privilege escalation inside userspace. In this case, the affected files are in debugfs (and should therefore only be accessible to root) and check that *pos is zero (which prevents the sys_splice() trick). Therefore, this is not a security fix, but rather a small cleanup. For the read handlers, fix it by using simple_read_from_buffer() instead of custom logic. For the write handler, add a check. changed in v2: - also fix dbg_write() Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters") Signed-off-by: Jann Horn Reviewed-by: Leon Romanovsky --- drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 28 +++++-------------- .../net/ethernet/mellanox/mlx5/core/debugfs.c | 22 ++------------- 2 files changed, 9 insertions(+), 41 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c index 384c1fa49081..92578074145d 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c @@ -1022,7 +1022,10 @@ static ssize_t dbg_write(struct file *filp, const char __user *buf, if (!dbg->in_msg || !dbg->out_msg) return -ENOMEM; - if (copy_from_user(lbuf, buf, sizeof(lbuf))) + if (count < sizeof(lbuf) - 1) + return -EINVAL; + + if (copy_from_user(lbuf, buf, sizeof(lbuf) - 1)) return -EFAULT; lbuf[sizeof(lbuf) - 1] = 0; @@ -1226,21 +1229,12 @@ static ssize_t data_read(struct file *filp, char __user *buf, size_t count, { struct mlx5_core_dev *dev = filp->private_data; struct mlx5_cmd_debug *dbg = &dev->cmd.dbg; - int copy; - - if (*pos) - return 0; if (!dbg->out_msg) return -ENOMEM; - copy = min_t(int, count, dbg->outlen); - if (copy_to_user(buf, dbg->out_msg, copy)) - return -EFAULT; - - *pos += copy; - - return copy; + return simple_read_from_buffer(buf, count, pos, dbg->out_msg, + dbg->outlen); } static const struct file_operations dfops = { @@ -1258,19 +1252,11 @@ static ssize_t outlen_read(struct file *filp, char __user *buf, size_t count, char outlen[8]; int err; - if (*pos) - return 0; - err = snprintf(outlen, sizeof(outlen), "%d", dbg->outlen); if (err < 0) return err; - if (copy_to_user(buf, &outlen, err)) - return -EFAULT; - - *pos += err; - - return err; + return simple_read_from_buffer(buf, count, pos, outlen, err); } static ssize_t outlen_write(struct file *filp, const char __user *buf, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c index 413080a312a7..90fabd612b6c 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c @@ -150,22 +150,13 @@ static ssize_t average_read(struct file *filp, char __user *buf, size_t count, int ret; char tbuf[22]; - if (*pos) - return 0; - stats = filp->private_data; spin_lock_irq(&stats->lock); if (stats->n) field = div64_u64(stats->sum, stats->n); spin_unlock_irq(&stats->lock); ret = snprintf(tbuf, sizeof(tbuf), "%llu\n", field); - if (ret > 0) { - if (copy_to_user(buf, tbuf, ret)) - return -EFAULT; - } - - *pos += ret; - return ret; + return simple_read_from_buffer(buf, count, pos, tbuf, ret); } static ssize_t average_write(struct file *filp, const char __user *buf, @@ -442,9 +433,6 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count, u64 field; int ret; - if (*pos) - return 0; - desc = filp->private_data; d = (void *)(desc - desc->i) - sizeof(*d); switch (d->type) { @@ -470,13 +458,7 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count, else ret = snprintf(tbuf, sizeof(tbuf), "0x%llx\n", field); - if (ret > 0) { - if (copy_to_user(buf, tbuf, ret)) - return -EFAULT; - } - - *pos += ret; - return ret; + return simple_read_from_buffer(buf, count, pos, tbuf, ret); } static const struct file_operations fops = {