[PATCH v2] usb: gadget: f_phonet: don't call gether_get_ifname() on a non-u_ether netdev

Alexander Bendezu posted 1 patch 1 week, 5 days ago
drivers/usb/gadget/function/f_phonet.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH v2] usb: gadget: f_phonet: don't call gether_get_ifname() on a non-u_ether netdev
Posted by Alexander Bendezu 1 week, 5 days ago
The f_phonet_ifname_show() function incorrectly used gether_get_ifname(),
which casts the net_device private data to 'struct eth_dev'. Since the
Phonet gadget only allocates a small 'struct phonet_port' for its private
data, this resulted in a KASAN slab-out-of-bounds read when accessing
dev->ifname_set.

  BUG: KASAN: slab-out-of-bounds in gether_get_ifname+0xda/0x100
  Read of size 1 at addr ffff8880091feaea by task cat/190

  Call Trace:
   <TASK>
   dump_stack_lvl+0x4d/0x70
   print_report+0x153/0x4c6
   kasan_report+0xda/0x110
   gether_get_ifname+0xda/0x100
   f_phonet_ifname_show+0x3a/0x60
   configfs_read_iter+0x2ea/0x600
   vfs_read+0x6da/0xa40
   ksys_read+0xfd/0x200
   do_syscall_64+0xe0/0x5a0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

  Allocated by task 189:
   kasan_save_stack+0x30/0x50
   kasan_save_track+0x14/0x30
   __kasan_kmalloc+0x7f/0x90
   __kvmalloc_node_noprof+0x1c2/0x5b0
   alloc_netdev_mqs+0x78/0x12d0
   phonet_alloc_inst+0x9e/0x1d0
   try_get_usb_function_instance+0xf9/0x1a0
   usb_get_function_instance+0xd/0x50
   function_make+0x163/0x340
   configfs_mkdir+0x47d/0xfc0

  The buggy address is located 154 bytes to the right of
   allocated 2640-byte region [ffff8880091fe000, ffff8880091fea50)

Fix this by reading the network device name directly with sysfs_emit(),
avoiding the invalid struct cast. The u_ether.h include is no longer
needed and is dropped.

No locking is needed: opts->net is established before the config group is
initialised, so the attribute cannot exist without a valid netdev, and
holding rtnl_lock() across the read would not prevent a rename from taking
effect before userspace sees the buffer.

It went unnoticed because the helper function is reached only through
USB_ETHERNET_CONFIGFS_ITEM_ATTR_IFNAME(), which is used exclusively by
u_ether functions, so phonet was the only caller passing a netdev u_ether
did not create.

Fixes: 83408745b202 ("usb: gadget: f_phonet: add configfs support")
Reported-by: syzbot+3a0d6aa450317f25e501@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3a0d6aa450317f25e501
Signed-off-by: Alexander Bendezu <alexanderbendezu10@gmail.com>
---
v2:
 - use sysfs_emit() instead of scnprintf() (Greg KH)
 - drop rtnl_lock(); the netdev cannot go away while the attribute
   exists, and holding it across the read would not prevent a rename
   from taking effect before userspace reads the buffer (Greg KH)
 - rewrite the commit message to describe the type confusion rather
   than the symptom
 - drop the now-unused u_ether.h include

 drivers/usb/gadget/function/f_phonet.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b1ee9a7c2e94..2c6558b1a2ff 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -23,7 +23,6 @@
 #include <linux/usb/composite.h>
 
 #include "u_phonet.h"
-#include "u_ether.h"
 
 #define PN_MEDIA_USB	0x1B
 #define MAXPACKET	512
@@ -600,7 +599,9 @@ static const struct configfs_item_operations phonet_item_ops = {
 
 static ssize_t f_phonet_ifname_show(struct config_item *item, char *page)
 {
-	return gether_get_ifname(to_f_phonet_opts(item)->net, page, PAGE_SIZE);
+	struct net_device *net = to_f_phonet_opts(item)->net;
+
+	return sysfs_emit(page, "%s\n", netdev_name(net));
 }
 
 CONFIGFS_ATTR_RO(f_phonet_, ifname);
-- 
2.53.0