From patchwork Fri Jul 6 15:20:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 940555 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="UF5B6eA1"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41Mdht4CKzz9s4V for ; Sat, 7 Jul 2018 01:20:46 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932851AbeGFPUf (ORCPT ); Fri, 6 Jul 2018 11:20:35 -0400 Received: from mail-qk0-f202.google.com ([209.85.220.202]:44215 "EHLO mail-qk0-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753759AbeGFPUd (ORCPT ); Fri, 6 Jul 2018 11:20:33 -0400 Received: by mail-qk0-f202.google.com with SMTP id w126-v6so4158834qka.11 for ; Fri, 06 Jul 2018 08:20:33 -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=GsOMmntGzvMQ0imlqQ8OOQNlMQcc0X+uUDdYN1l/Hyg=; b=UF5B6eA11m9dtBGlHZxIef60K2+9jKDeviqgdoZ9ijY64jsw9BAsISvo0CviCk8b+P UbTwdJTzlAwEe2Eb6gbowubao9IBuXn+TGfPSuhHRtcij7Jsu9Qqv6ojo24SZD5Jucqa s72q/uA1NpXknTBIWoWBuBmbzUTw817VEJVKrxZov0DL4YekFPjRnzdvXzrwO8Q4P8do 4Y3NSIMFFhQZu4m+OQWdGUNA7emWyBaP6wtlyoAgAUxerJxGUFQZ7zx5AZI5ARQLhXjT IDflZjfjdB0ooUsP8WGhaTe+WysyC3u4EmCkWYswQQDttn3XBNB8DeZnH/QIZOcny7lJ +C1w== 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=GsOMmntGzvMQ0imlqQ8OOQNlMQcc0X+uUDdYN1l/Hyg=; b=AjsQSzfQTjRmRX91qevpXVCSm8PU94+OeOlgybniayeap7paUZZCmwp9rZXYBiCQMm 0MeiZ4Cb2W59T2DpmFlGpvjT3l+UI4/s2xw3f/gYi1TUPGXev3u/aE51x2tfzgZqFbmI KmCH65ePcqMRsmaWn9LlK7cqetCm043pgCilCsHhC6WMS0Ai6L3xWmRayNkpTVZqGrEQ ipiUu8/ALbb5E7STx8r9LSWoICua+4tukRU4eCvsnP+c9+gnp4KupT7uZ0Gauaq07kEu xwxSjZn1fVY5wZsMYrQnd/5MTL1yOzILFPIgjQjIDWONTYxtXSJtZqZyPLmS9GnqoIe8 I5nA== X-Gm-Message-State: APt69E3qeOsoALcWtettwwqXT0MoDeXkfF+DzQcev+NCLhgm2z6Yotu5 2xyxPUuiMn2tjCWTGxNsgBD71MN64w== X-Google-Smtp-Source: AAOMgpcoE2uA2SVxza6gguyKoPVWO03IModdlm/aU9WBB1j9mfhigyzLq5btWcJcg9rd0Gjs8myfw1vMmg== MIME-Version: 1.0 X-Received: by 2002:ac8:7103:: with SMTP id z3-v6mr6281887qto.50.1530890433129; Fri, 06 Jul 2018 08:20:33 -0700 (PDT) Date: Fri, 6 Jul 2018 17:20:28 +0200 Message-Id: <20180706152028.31775-1-jannh@google.com> X-Mailer: git-send-email 2.18.0.399.gad0ab374a1-goog Subject: [PATCH net] net/mlx5: fix uaccess beyond "count" in debugfs read 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. Fix it by using simple_read_from_buffer() instead of custom logic. Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters") Signed-off-by: Jann Horn --- drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 23 +++---------------- .../net/ethernet/mellanox/mlx5/core/debugfs.c | 22 ++---------------- 2 files changed, 5 insertions(+), 40 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c index 384c1fa49081..0a79a4540923 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c @@ -1226,21 +1226,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 +1249,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 = {