lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YE3dMM7tqk5BbD/l@shredder.lan>
Date:   Sun, 14 Mar 2021 11:53:52 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        lukasz.luba@....com, Jiri Pirko <jiri@...dia.com>,
        Ido Schimmel <idosch@...dia.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Amit Kucheria <amitk@...nel.org>,
        "open list:MELLANOX ETHERNET SWITCH DRIVERS" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 1/5] thermal/drivers/core: Use a char pointer for the
 cooling device name

On Fri, Mar 12, 2021 at 06:03:12PM +0100, Daniel Lezcano wrote:
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 996c038f83a4..9ef8090eb645 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -960,10 +960,7 @@ __thermal_cooling_device_register(struct device_node *np,
>  {
>  	struct thermal_cooling_device *cdev;
>  	struct thermal_zone_device *pos = NULL;
> -	int result;
> -
> -	if (type && strlen(type) >= THERMAL_NAME_LENGTH)
> -		return ERR_PTR(-EINVAL);
> +	int ret;
>  
>  	if (!ops || !ops->get_max_state || !ops->get_cur_state ||
>  	    !ops->set_cur_state)
> @@ -973,14 +970,17 @@ __thermal_cooling_device_register(struct device_node *np,
>  	if (!cdev)
>  		return ERR_PTR(-ENOMEM);
>  
> -	result = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> -	if (result < 0) {
> -		kfree(cdev);
> -		return ERR_PTR(result);
> +	ret = ida_simple_get(&thermal_cdev_ida, 0, 0, GFP_KERNEL);
> +	if (ret < 0)
> +		goto out_kfree_cdev;
> +	cdev->id = ret;
> +
> +	cdev->type = kstrdup(type ? type : "", GFP_KERNEL);
> +	if (!cdev->type) {
> +		ret = -ENOMEM;
> +		goto out_ida_remove;
>  	}
>  
> -	cdev->id = result;
> -	strlcpy(cdev->type, type ? : "", sizeof(cdev->type));
>  	mutex_init(&cdev->lock);
>  	INIT_LIST_HEAD(&cdev->thermal_instances);
>  	cdev->np = np;
> @@ -990,12 +990,9 @@ __thermal_cooling_device_register(struct device_node *np,
>  	cdev->devdata = devdata;
>  	thermal_cooling_device_setup_sysfs(cdev);
>  	dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
> -	result = device_register(&cdev->device);
> -	if (result) {
> -		ida_simple_remove(&thermal_cdev_ida, cdev->id);
> -		put_device(&cdev->device);
> -		return ERR_PTR(result);
> -	}
> +	ret = device_register(&cdev->device);
> +	if (ret)
> +		goto out_kfree_type;
>  
>  	/* Add 'this' new cdev to the global cdev list */
>  	mutex_lock(&thermal_list_lock);
> @@ -1013,6 +1010,14 @@ __thermal_cooling_device_register(struct device_node *np,
>  	mutex_unlock(&thermal_list_lock);
>  
>  	return cdev;
> +
> +out_kfree_type:
> +	kfree(cdev->type);
> +	put_device(&cdev->device);
> +out_ida_remove:
> +	ida_simple_remove(&thermal_cdev_ida, cdev->id);
> +out_kfree_cdev:
> +	return ERR_PTR(ret);
>  }
>  
>  /**
> @@ -1172,6 +1177,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
>  	device_del(&cdev->device);
>  	thermal_cooling_device_destroy_sysfs(cdev);
>  	put_device(&cdev->device);
> +	kfree(cdev->type);
>  }
>  EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);

I'm getting the following user-after-free with this patch [1]. Fixed by:

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 9ef8090eb645..c8d4010940ef 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1176,8 +1176,8 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
        ida_simple_remove(&thermal_cdev_ida, cdev->id);
        device_del(&cdev->device);
        thermal_cooling_device_destroy_sysfs(cdev);
-       put_device(&cdev->device);
        kfree(cdev->type);
+       put_device(&cdev->device);
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);

[1]
[  148.601815] ==================================================================
[  148.610260] BUG: KASAN: use-after-free in thermal_cooling_device_unregister+0x6ca/0x6e0
[  148.619304] Read of size 8 at addr ffff8881510a0808 by task devlink/574
[  148.626768]
[  148.628477] CPU: 2 PID: 574 Comm: devlink Not tainted 5.12.0-rc2-custom-12525-g7ba8a2feee15 #3301
[  148.638463] Hardware name: Mellanox Technologies Ltd. MSN2100-CB2FO/SA001017, BIOS 5.6.5 06/07/2016
[  148.648625] Call Trace:
[  148.651408]  dump_stack+0xfa/0x151
[  148.661701]  print_address_description.constprop.0+0x18/0x130
[  148.681014]  kasan_report.cold+0x7f/0x111
[  148.692003]  thermal_cooling_device_unregister+0x6ca/0x6e0
[  148.703984]  mlxsw_thermal_fini+0xd2/0x1f0
[  148.708664]  mlxsw_core_bus_device_unregister+0x158/0x8d0
[  148.714794]  mlxsw_devlink_core_bus_device_reload_down+0x93/0xc0
[  148.721594]  devlink_reload+0x15f/0x5e0
[  148.749669]  devlink_nl_cmd_reload+0x7fc/0x1210
[  148.775992]  genl_family_rcv_msg_doit+0x22a/0x320
[  148.799789]  genl_rcv_msg+0x341/0x5a0
[  148.818789]  netlink_rcv_skb+0x14d/0x430
[  148.836450]  genl_rcv+0x29/0x40
[  148.840034]  netlink_unicast+0x539/0x7e0
[  148.859219]  netlink_sendmsg+0x8d7/0xe10
[  148.879271]  __sys_sendto+0x23f/0x350
[  148.904178]  __x64_sys_sendto+0xe2/0x1b0
[  148.919297]  do_syscall_64+0x2d/0x40
[  148.923365]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  148.929081] RIP: 0033:0x7f17c0dbaefa
[  148.933139] Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 76
 c3 0f 1f 44 00 00 55 48 83 ec 30 44 89 4c
[  148.954190] RSP: 002b:00007ffd879c5e18 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[  148.962723] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f17c0dbaefa
[  148.970751] RDX: 0000000000000030 RSI: 0000000000ad0ad0 RDI: 0000000000000003
[  148.978776] RBP: 0000000000ad0aa0 R08: 00007f17c0e8d000 R09: 000000000000000c
[  148.986803] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000408d70
[  148.994834] R13: 0000000000ad0910 R14: 0000000000000000 R15: 0000000000000001
[  149.002978]
[  149.004687] Allocated by task 1:
[  149.008345]  kasan_save_stack+0x1b/0x40
[  149.012711]  __kasan_kmalloc+0x7a/0x90
[  149.016974]  __thermal_cooling_device_register.part.0+0x59/0x9e0
[  149.023753]  thermal_cooling_device_register+0xb3/0x100
[  149.029671]  mlxsw_thermal_init+0x78b/0xa10
[  149.034427]  __mlxsw_core_bus_device_register+0xd05/0x1a30
[  149.040634]  mlxsw_core_bus_device_register+0x56/0xb0
[  149.046349]  mlxsw_pci_probe+0x53b/0x750
[  149.050800]  local_pci_probe+0xc6/0x170
[  149.055144]  pci_device_probe+0x2a3/0x4a0
[  149.059683]  really_probe+0x2b6/0xec0
[  149.063840]  driver_probe_device+0x1e2/0x330
[  149.068673]  device_driver_attach+0x282/0x2f0
[  149.073605]  __driver_attach+0x160/0x2f0
[  149.078050]  bus_for_each_dev+0x14c/0x1d0
[  149.082589]  bus_add_driver+0x3ac/0x650
[  149.086935]  driver_register+0x225/0x3a0
[  149.091381]  mlxsw_sp_module_init+0xa2/0x174
[  149.096216]  do_one_initcall+0x108/0x690
[  149.100660]  kernel_init_freeable+0x3ec/0x46b
[  149.105595]  kernel_init+0x13/0x1eb
[  149.109559]  ret_from_fork+0x1f/0x30
[  149.113613]
[  149.115311] Freed by task 574:
[  149.118765]  kasan_save_stack+0x1b/0x40
[  149.123116]  kasan_set_track+0x1c/0x30
[  149.127373]  kasan_set_free_info+0x20/0x30
[  149.132021]  __kasan_slab_free+0xe5/0x110
[  149.136556]  slab_free_freelist_hook+0x59/0x150
[  149.141681]  kfree+0xd5/0x3b0
[  149.145055]  thermal_release+0xa0/0x110
[  149.149414]  device_release+0xa4/0x240
[  149.153680]  kobject_put+0x1c8/0x540
[  149.157747]  put_device+0x20/0x30
[  149.161530]  thermal_cooling_device_unregister+0x578/0x6e0
[  149.167751]  mlxsw_thermal_fini+0xd2/0x1f0
[  149.172414]  mlxsw_core_bus_device_unregister+0x158/0x8d0
[  149.178529]  mlxsw_devlink_core_bus_device_reload_down+0x93/0xc0
[  149.185327]  devlink_reload+0x15f/0x5e0
[  149.189695]  devlink_nl_cmd_reload+0x7fc/0x1210
[  149.194838]  genl_family_rcv_msg_doit+0x22a/0x320
[  149.200182]  genl_rcv_msg+0x341/0x5a0
[  149.204337]  netlink_rcv_skb+0x14d/0x430
[  149.208799]  genl_rcv+0x29/0x40
[  149.212381]  netlink_unicast+0x539/0x7e0
[  149.216825]  netlink_sendmsg+0x8d7/0xe10
[  149.221274]  __sys_sendto+0x23f/0x350
[  149.225423]  __x64_sys_sendto+0xe2/0x1b0
[  149.229864]  do_syscall_64+0x2d/0x40
[  149.233912]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  149.239624]
[  149.241322] The buggy address belongs to the object at ffff8881510a0800
[  149.241322]  which belongs to the cache kmalloc-2k of size 2048
[  149.255372] The buggy address is located 8 bytes inside of
[  149.255372]  2048-byte region [ffff8881510a0800, ffff8881510a1000)
[  149.268456] The buggy address belongs to the page:
[  149.273850] page:000000006ec87a73 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1510a0
[  149.284416] head:000000006ec87a73 order:3 compound_mapcount:0 compound_pincount:0
[  149.292838] flags: 0x200000000010200(slab|head)
[  149.297981] raw: 0200000000010200 ffffea000426e208 ffffea000544b808 ffff88810004de40
[  149.306707] raw: 0000000000000000 0000000000050005 00000001ffffffff 0000000000000000
[  149.315417] page dumped because: kasan: bad access detected
[  149.321697]
[  149.323403] Memory state around the buggy address:
[  149.328811]  ffff8881510a0700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  149.336939]  ffff8881510a0780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  149.345075] >ffff8881510a0800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  149.353202]                       ^
[  149.357159]  ffff8881510a0880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  149.365298]  ffff8881510a0900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  149.373424] ==================================================================

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ