All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
	David Miller <davem@davemloft.net>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com, Bruce Allan <bruce.w.allan@intel.com>,
	Ben Hutchings <ben@decadent.org.uk>
Subject: [PATCH] ethtool: Use kcalloc instead of kmalloc for ethtool_get_strings
Date: Wed, 14 Oct 2015 01:09:40 -0700	[thread overview]
Message-ID: <1444810180.2718.16.camel@perches.com> (raw)
In-Reply-To: <1444805996-3877-16-git-send-email-jeffrey.t.kirsher@intel.com>

It seems that kernel memory can leak into userspace by a
kmalloc, ethtool_get_strings, then copy_to_user sequence.

Avoid this by using kcalloc to zero fill the copied buffer.

Signed-off-by: Joe Perches <joe@perches.com>
---

stable too...

On Tue, 2015-10-13 at 23:59 -0700, Jeff Kirsher wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
[]
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
[]
> @@ -206,13 +206,13 @@ static void fm10k_get_stat_strings(struct net_device *dev, u8 *data)
>  	}
>  
>  	for (i = 0; i < interface->hw.mac.max_queues; i++) {
> -		sprintf(p, "tx_queue_%u_packets", i);
> +		snprintf(p, ETH_GSTRING_LEN, "tx_queue_%u_packets", i);

It seems these need a memset after the snprintf to zero fill
bytes after the string terminating \0 to avoid leaking
contents of any unset bytes.

It'd probably be better to allocate a zeroed buffer instead.

>  		p += ETH_GSTRING_LEN;
> -		sprintf(p, "tx_queue_%u_bytes", i);
> +		snprintf(p, ETH_GSTRING_LEN, "tx_queue_%u_bytes", i);

so...

 net/core/ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index b495ab1..29edf74 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1284,7 +1284,7 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 
 	gstrings.len = ret;
 
-	data = kmalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
+	data = kcalloc(gstrings.len, ETH_GSTRING_LEN, GFP_USER);
 	if (!data)
 		return -ENOMEM;
 

  reply	other threads:[~2015-10-14  8:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14  6:59 [net-next v2 00/16][pull request] Intel Wired LAN Driver Updates 2015-10-13 Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 01/16] i40e/i40evf: Add new link status defines Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 02/16] i40e: Make it clear a parameter is never used Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 03/16] i40e: Use BIT() macro for priority map parsing Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 04/16] i40evf: properly handle ndo_set_mac_address calls Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 05/16] i40e/i40evf: Add info to nvm info struct for OEM version data Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 06/16] i40e: Convert CEE App TLV selector to IEEE selector Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 07/16] i40e: remove redundant call Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 08/16] i40e: don't panic on VSI allocation failure Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 09/16] i40e: update fw version text string per previous product formats Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 10/16] i40e/i40evf: split device ids into a separate file Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 11/16] i40e/i40evf: Add module_types and update_link_info Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 12/16] i40e/i40evf: Refactor PHY structure and add phy_capabilities enum Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 13/16] i40e/i40evf: Bump i40e version to 1.3.25 and i40evf to 1.3.17 Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 14/16] ixgbe: add flow control ethertype to the anti-spoofing filter Jeff Kirsher
2015-10-14  6:59 ` [net-next v2 15/16] fm10k: use snprintf() instead of sprintf() to avoid buffer overflow Jeff Kirsher
2015-10-14  8:09   ` Joe Perches [this message]
2015-10-14 20:36     ` [PATCH] ethtool: Use kcalloc instead of kmalloc for ethtool_get_strings Ben Hutchings
2015-10-15  2:01     ` David Miller
2015-10-14  6:59 ` [net-next v2 16/16] fm10k: do not use enum as boolean Jeff Kirsher
2015-10-14 12:54 ` [net-next v2 00/16][pull request] Intel Wired LAN Driver Updates 2015-10-13 David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1444810180.2718.16.camel@perches.com \
    --to=joe@perches.com \
    --cc=ben@decadent.org.uk \
    --cc=bruce.w.allan@intel.com \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jogreene@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@redhat.com \
    --cc=sassmann@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.