bna: integer overflow bug in debugfs

PATCH Applied Dan Carpenter 3 participants 3 messages Mar 17, 2017 – Mar 22, 2017

Subsystems (4)

Networking Network drivers Kernel janitors Brocade bna 10 gigabit Ethernet driver

lore

Share & tools

lore

Dan Carpenter

author

2017-03-17 20:52 UTC #0 lore raw

PATCH 1/1 bna: integer overflow bug in debugfs

We could allocate less memory than intended because we do:

	bnad->regdata = kzalloc(len << 2, GFP_KERNEL);

The shift can overflow leading to a crash. This is debugfs code so the impact is very small.

Fixes 7afc5dbde091 ("bna: Add debugfs interface.")

Signed-off-by Dan Carpenter <[email protected]>

drivers/net/ethernet/brocade/bna/bnad_debugfs.c +1 -1

index 05c1c1dd7751..cebfe3bd086e 100644
@@ -325,7 +325,7 @@ bnad_debugfs_write_regrd(struct file *file, const char __user *buf,
 		return PTR_ERR(kern_buf);
 
 	rc = sscanf(kern_buf, "%x:%x", &addr, &len);
-	if (rc < 2) {
+	if (rc < 2 || len > UINT_MAX >> 2) {
 		netdev_warn(bnad->netdev, "failed to read user buffer\n");
 		kfree(kern_buf);
 		return -EINVAL;

Mody, Rasesh

Acked-by on v1 1/1

2017-03-21 01:14 UTC #1 lore raw

Quoted 27 lines

From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, March 17, 2017 1:53 PM
We could allocate less memory than intended because we do:
	bnad->regdata = kzalloc(len << 2, GFP_KERNEL);
The shift can overflow leading to a crash. This is debugfs code so the impact is very small.
Fixes 7afc5dbde091 ("bna: Add debugfs interface.")
Signed-off-by Dan Carpenter <[email protected]>
drivers/net/ethernet/brocade/bna/bnad_debugfs.c +1 -1
index 05c1c1dd7751..cebfe3bd086e 100644
@@ -325,7 +325,7 @@ bnad_debugfs_write_regrd(struct file *file, const
 char __user *buf,
 		return PTR_ERR(kern_buf);
 
 	rc = sscanf(kern_buf, "%x:%x", &addr, &len);
-	if (rc < 2) {
+	if (rc < 2 || len > UINT_MAX >> 2) {
 		netdev_warn(bnad->netdev, "failed to read user buffer\n");
 		kfree(kern_buf);

Quoted 1 line

return -EINVAL;

You are correct, thanks Dan for adding the check.

Acked-by Rasesh Mody <[email protected]>

David Miller

maintainer Applied

2017-03-22 00:43 UTC #2 lore raw

In reply to Dan Carpenter

From: Dan Carpenter <[email protected]>
Date: Fri, 17 Mar 2017 23:52:35 +0300

Quoted 9 lines

We could allocate less memory than intended because we do:
	bnad->regdata = kzalloc(len << 2, GFP_KERNEL);
The shift can overflow leading to a crash. This is debugfs code so the impact is very small.
Fixes 7afc5dbde091 ("bna: Add debugfs interface.")
Signed-off-by Dan Carpenter <[email protected]>

Applied.