Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/532239/?format=api
{ "id": 532239, "url": "http://patchwork.ozlabs.org/api/patches/532239/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1445115499-28728-1-git-send-email-linux@rasmusvillemoes.dk/", "project": { "id": 46, "url": "http://patchwork.ozlabs.org/api/projects/46/?format=api", "name": "Intel Wired Ethernet development", "link_name": "intel-wired-lan", "list_id": "intel-wired-lan.osuosl.org", "list_email": "intel-wired-lan@osuosl.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<1445115499-28728-1-git-send-email-linux@rasmusvillemoes.dk>", "list_archive_url": null, "date": "2015-10-17T20:58:19", "name": "intel: i40e: fix confused code", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "ada0380a3a8abd100e3972d0e3d4c8b9476657e2", "submitter": { "id": 27394, "url": "http://patchwork.ozlabs.org/api/people/27394/?format=api", "name": "Rasmus Villemoes", "email": "linux@rasmusvillemoes.dk" }, "delegate": { "id": 68, "url": "http://patchwork.ozlabs.org/api/users/68/?format=api", "username": "jtkirshe", "first_name": "Jeff", "last_name": "Kirsher", "email": "jeffrey.t.kirsher@intel.com" }, "mbox": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1445115499-28728-1-git-send-email-linux@rasmusvillemoes.dk/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/532239/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/532239/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<intel-wired-lan-bounces@lists.osuosl.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Received": [ "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\tby ozlabs.org (Postfix) with ESMTP id CAA171402D2\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 19 Oct 2015 22:34:25 +1100 (AEDT)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 02C9F8A462;\n\tMon, 19 Oct 2015 11:34:25 +0000 (UTC)", "from hemlock.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id sGxR99LGaRH7; Mon, 19 Oct 2015 11:34:23 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 0885A8A3C6;\n\tMon, 19 Oct 2015 11:34:23 +0000 (UTC)", "from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n\tby ash.osuosl.org (Postfix) with ESMTP id 7B7631C44C2\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tSat, 17 Oct 2015 21:05:17 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby whitealder.osuosl.org (Postfix) with ESMTP id 70A9789BAA\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tSat, 17 Oct 2015 21:05:17 +0000 (UTC)", "from whitealder.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id U1lAIIR2sZhi for <intel-wired-lan@lists.osuosl.org>;\n\tSat, 17 Oct 2015 21:05:16 +0000 (UTC)", "from mail-lb0-f174.google.com (mail-lb0-f174.google.com\n\t[209.85.217.174])\n\tby whitealder.osuosl.org (Postfix) with ESMTPS id F045789B63\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tSat, 17 Oct 2015 21:05:15 +0000 (UTC)", "by lbbwb3 with SMTP id wb3so58481527lbb.1\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tSat, 17 Oct 2015 14:05:14 -0700 (PDT)", "from garcia.imf.au.dk ([130.225.20.51])\n\tby smtp.gmail.com with ESMTPSA id\n\tg140sm3964351lfg.29.2015.10.17.13.59.09\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tSat, 17 Oct 2015 13:59:09 -0700 (PDT)" ], "Authentication-Results": "ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=rasmusvillemoes.dk header.i=@rasmusvillemoes.dk\n\theader.b=Bz0GM7Ly; dkim-atps=neutral", "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "delayed 00:06:03 by SQLgrey-1.7.6", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=rasmusvillemoes.dk; s=google;\n\th=from:to:cc:subject:date:message-id;\n\tbh=xJdyKP6IBSAzwhE66a2D0qJxVSINiKsSxustJAHnA3U=;\n\tb=Bz0GM7LyQV1Enr9vGjt1k3D1Lc3uE2CTDMYHZxFuB6tEEMdUztdygHJFt7vQbdOZxU\n\tONkj2o6s3BZDxkE0k5TCHtamm/wWJ+vRFc8JsR9x5HmceBfgli36sOsWr66KzDtJLZqS\n\tZobMlF4bgxOK/kr6M/9fnZy8n9qDuCDYuEpI0=", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id;\n\tbh=xJdyKP6IBSAzwhE66a2D0qJxVSINiKsSxustJAHnA3U=;\n\tb=KWBYm0TgBpL60xIYj6AcF0d1PcwhspJOw5MkgDWkJVsSVGsM6nQM895zujHvqdN+eE\n\tLugohK+mc6TcRqUaTxKmyp6imwMFU+5y2F2Hg8pDP2CTJ/GeY0kUfwp3WyTwLWFL6eoh\n\tBQoGu0gbhUqUvc9JCeqn+XaytuEKxgbzhrHFxRfAb2b7xuNEgvWfk6yYqN9f3Ix30zZf\n\tgsFjNnSjd+c3ZitOD4bJ4zzElP6yd24TnFKPUXB55jEmqGoWdAOIVGnOwY+RR77tm4iu\n\ty3GH4zG/xmrwhcIazB7rdvLUrCDkdv4/9ZzBRJe3oC7t2aavwD08yJbDzbVHcxeC1HOx\n\tO4dQ==", "X-Gm-Message-State": "ALoCoQn9tgO5lEDcJ5KYE49XuiJuyO6OGWOvLWqX8cVloSF4+MXClM8d6NhjvNIi67T5tF27cDzv", "X-Received": "by 10.112.135.233 with SMTP id pv9mr5337612lbb.42.1445115550712; \n\tSat, 17 Oct 2015 13:59:10 -0700 (PDT)", "From": "Rasmus Villemoes <linux@rasmusvillemoes.dk>", "To": "Jeff Kirsher <jeffrey.t.kirsher@intel.com>,\n\tJesse Brandeburg <jesse.brandeburg@intel.com>,\n\tShannon Nelson <shannon.nelson@intel.com>,\n\tCarolyn Wyborny <carolyn.wyborny@intel.com>,\n\tDon Skidmore <donald.c.skidmore@intel.com>,\n\tMatthew Vick <matthew.vick@intel.com>,\n\tJohn Ronciak <john.ronciak@intel.com>,\n\tMitch Williams <mitch.a.williams@intel.com>", "Date": "Sat, 17 Oct 2015 22:58:19 +0200", "Message-Id": "<1445115499-28728-1-git-send-email-linux@rasmusvillemoes.dk>", "X-Mailer": "git-send-email 2.6.1", "X-Mailman-Approved-At": "Mon, 19 Oct 2015 11:34:21 +0000", "Cc": "intel-wired-lan@lists.osuosl.org,\n\tRasmus Villemoes <linux@rasmusvillemoes.dk>,\n\tlinux-kernel@vger.kernel.org, netdev@vger.kernel.org", "Subject": "[Intel-wired-lan] [PATCH] intel: i40e: fix confused code", "X-BeenThere": "intel-wired-lan@lists.osuosl.org", "X-Mailman-Version": "2.1.18-1", "Precedence": "list", "List-Id": "Intel Wired Ethernet Linux Kernel Driver Development\n\t<intel-wired-lan.lists.osuosl.org>", "List-Unsubscribe": "<http://lists.osuosl.org/mailman/options/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=unsubscribe>", "List-Archive": "<http://lists.osuosl.org/pipermail/intel-wired-lan/>", "List-Post": "<mailto:intel-wired-lan@lists.osuosl.org>", "List-Help": "<mailto:intel-wired-lan-request@lists.osuosl.org?subject=help>", "List-Subscribe": "<http://lists.osuosl.org/mailman/listinfo/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=subscribe>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "intel-wired-lan-bounces@lists.osuosl.org", "Sender": "\"Intel-wired-lan\" <intel-wired-lan-bounces@lists.osuosl.org>" }, "content": "This code is pretty confused. The variable name 'bytes_not_copied'\nclearly indicates that the programmer knew the semantics of\ncopy_{to,from}_user, but then the return value is checked for being\nnegative and used as a -Exxx return value.\n\nI'm not sure this is the proper fix, but at least we get rid of the\ndead code which pretended to check for access faults.\n\nSigned-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>\n---\nThere are other things worth looking at. i40e_dbg_netdev_ops_buf is a\nstatic buffer of size 256, which can be filled from user space (in\ni40e_dbg_netdev_ops_write). That function correctly checks that the\ninput is at most 255 bytes. However, in i40e_dbg_netdev_ops_read we\nsnprintf() it along a device name (and some white space) into\nkmalloc'ed buffer, also of size 256. Hence the snprintf output can be\ntruncated, but snprintf() returns the total size that would be\ngenerated - so when we then proceed to using that in copy_to_user(),\nwe may actually copy from beyond the allocated buffer, hence leaking a\nlittle kernel data.\n\nIn i40e_dbg_command_write, we allocate a buffer based on count which\nis user-supplied. While kmalloc() refuses completely insane sizes, we\nmay still allocate a few MB. Moreover, if allocation fails, returning\n'count' is rather odd; -ENOMEM would make more sense.\n\n drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 24 ++++++++++--------------\n 1 file changed, 10 insertions(+), 14 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c\nindex d7c15d17faa6..e9ecd3f9cafe 100644\n--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c\n+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c\n@@ -103,8 +103,8 @@ static ssize_t i40e_dbg_dump_read(struct file *filp, char __user *buffer,\n \tlen = min_t(int, count, (i40e_dbg_dump_data_len - *ppos));\n \n \tbytes_not_copied = copy_to_user(buffer, &i40e_dbg_dump_buf[*ppos], len);\n-\tif (bytes_not_copied < 0)\n-\t\treturn bytes_not_copied;\n+\tif (bytes_not_copied)\n+\t\treturn -EFAULT;\n \n \t*ppos += len;\n \treturn len;\n@@ -353,8 +353,8 @@ static ssize_t i40e_dbg_command_read(struct file *filp, char __user *buffer,\n \tbytes_not_copied = copy_to_user(buffer, buf, len);\n \tkfree(buf);\n \n-\tif (bytes_not_copied < 0)\n-\t\treturn bytes_not_copied;\n+\tif (bytes_not_copied)\n+\t\treturn -EFAULT;\n \n \t*ppos = len;\n \treturn len;\n@@ -995,12 +995,10 @@ static ssize_t i40e_dbg_command_write(struct file *filp,\n \tif (!cmd_buf)\n \t\treturn count;\n \tbytes_not_copied = copy_from_user(cmd_buf, buffer, count);\n-\tif (bytes_not_copied < 0) {\n+\tif (bytes_not_copied) {\n \t\tkfree(cmd_buf);\n-\t\treturn bytes_not_copied;\n+\t\treturn -EFAULT;\n \t}\n-\tif (bytes_not_copied > 0)\n-\t\tcount -= bytes_not_copied;\n \tcmd_buf[count] = '\\0';\n \n \tcmd_buf_tmp = strchr(cmd_buf, '\\n');\n@@ -2034,8 +2032,8 @@ static ssize_t i40e_dbg_netdev_ops_read(struct file *filp, char __user *buffer,\n \tbytes_not_copied = copy_to_user(buffer, buf, len);\n \tkfree(buf);\n \n-\tif (bytes_not_copied < 0)\n-\t\treturn bytes_not_copied;\n+\tif (bytes_not_copied)\n+\t\treturn -EFAULT;\n \n \t*ppos = len;\n \treturn len;\n@@ -2068,10 +2066,8 @@ static ssize_t i40e_dbg_netdev_ops_write(struct file *filp,\n \tmemset(i40e_dbg_netdev_ops_buf, 0, sizeof(i40e_dbg_netdev_ops_buf));\n \tbytes_not_copied = copy_from_user(i40e_dbg_netdev_ops_buf,\n \t\t\t\t\t buffer, count);\n-\tif (bytes_not_copied < 0)\n-\t\treturn bytes_not_copied;\n-\telse if (bytes_not_copied > 0)\n-\t\tcount -= bytes_not_copied;\n+\tif (bytes_not_copied)\n+\t\treturn -EFAULT;\n \ti40e_dbg_netdev_ops_buf[count] = '\\0';\n \n \tbuf_tmp = strchr(i40e_dbg_netdev_ops_buf, '\\n');\n", "prefixes": [] }