linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] libertas: Avoid reading past end of buffer
@ 2017-05-15 21:26 Kees Cook
  2017-05-24 13:44 ` [v3] " Kalle Valo
  0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2017-05-15 21:26 UTC (permalink / raw)
  To: netdev
  Cc: Kalle Valo, Joe Perches, libertas-dev, linux-wireless, netdev,
	Daniel Micay, linux-kernel

Using memcpy() from a string that is shorter than the length copied means
the destination buffer is being filled with arbitrary data from the kernel
rodata segment. Instead, redefine the stat strings to be ETH_GSTRING_LEN
sizes, like other drivers. This lets us use a single memcpy that does not
leak rodata contents. Additionally adjust indentation to keep checkpatch.pl
happy.

This was found with the future CONFIG_FORTIFY_SOURCE feature.

Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v3:
- drop needless "*"; joe
- fix entry/exit in separate patch
v2:
- use ETH_GSTRING_LEN; joe
---
 drivers/net/wireless/marvell/libertas/mesh.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/mesh.c b/drivers/net/wireless/marvell/libertas/mesh.c
index d0c881dd5846..2229fb448189 100644
--- a/drivers/net/wireless/marvell/libertas/mesh.c
+++ b/drivers/net/wireless/marvell/libertas/mesh.c
@@ -1108,15 +1108,15 @@ void lbs_mesh_set_txpd(struct lbs_private *priv,
  * Ethtool related
  */
 
-static const char * const mesh_stat_strings[] = {
-			"drop_duplicate_bcast",
-			"drop_ttl_zero",
-			"drop_no_fwd_route",
-			"drop_no_buffers",
-			"fwded_unicast_cnt",
-			"fwded_bcast_cnt",
-			"drop_blind_table",
-			"tx_failed_cnt"
+static const char mesh_stat_strings[MESH_STATS_NUM][ETH_GSTRING_LEN] = {
+	"drop_duplicate_bcast",
+	"drop_ttl_zero",
+	"drop_no_fwd_route",
+	"drop_no_buffers",
+	"fwded_unicast_cnt",
+	"fwded_bcast_cnt",
+	"drop_blind_table",
+	"tx_failed_cnt"
 };
 
 void lbs_mesh_ethtool_get_stats(struct net_device *dev,
@@ -1170,17 +1170,11 @@ int lbs_mesh_ethtool_get_sset_count(struct net_device *dev, int sset)
 void lbs_mesh_ethtool_get_strings(struct net_device *dev,
 	uint32_t stringset, uint8_t *s)
 {
-	int i;
-
 	lbs_deb_enter(LBS_DEB_ETHTOOL);
 
 	switch (stringset) {
 	case ETH_SS_STATS:
-		for (i = 0; i < MESH_STATS_NUM; i++) {
-			memcpy(s + i * ETH_GSTRING_LEN,
-					mesh_stat_strings[i],
-					ETH_GSTRING_LEN);
-		}
+		memcpy(s, mesh_stat_strings, sizeof(mesh_stat_strings));
 		break;
 	}
 	lbs_deb_enter(LBS_DEB_ETHTOOL);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [v3] libertas: Avoid reading past end of buffer
  2017-05-15 21:26 [PATCH v3] libertas: Avoid reading past end of buffer Kees Cook
@ 2017-05-24 13:44 ` Kalle Valo
  0 siblings, 0 replies; 2+ messages in thread
From: Kalle Valo @ 2017-05-24 13:44 UTC (permalink / raw)
  To: Kees Cook
  Cc: netdev, Joe Perches, libertas-dev, linux-wireless, netdev,
	Daniel Micay, linux-kernel

Kees Cook <keescook@chromium.org> wrote:
> Using memcpy() from a string that is shorter than the length copied means
> the destination buffer is being filled with arbitrary data from the kernel
> rodata segment. Instead, redefine the stat strings to be ETH_GSTRING_LEN
> sizes, like other drivers. This lets us use a single memcpy that does not
> leak rodata contents. Additionally adjust indentation to keep checkpatch.pl
> happy.
> 
> This was found with the future CONFIG_FORTIFY_SOURCE feature.
> 
> Cc: Daniel Micay <danielmicay@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

12e3c0433e8a libertas: Avoid reading past end of buffer

-- 
https://patchwork.kernel.org/patch/9727997/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-05-24 13:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 21:26 [PATCH v3] libertas: Avoid reading past end of buffer Kees Cook
2017-05-24 13:44 ` [v3] " Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).