Bug 188451
| Summary: | Function lbs_cmd_802_11_sleep_params() returns an improper error code when the call to lbs_cmd_with_response() fails, which may result in stack information leak | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | RUC_Soft_Sec (rucsoftsec) |
| Component: | network-wireless | Assignee: | drivers_network-wireless (drivers_network-wireless) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | bianpan2010 |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | linux-4.9-rc6 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: | The patch fixes the bug | ||
Created attachment 256391 [details]
The patch fixes the bug
The patch has been merged into the latest version of the Linux kernel. So I will close it.
|
Function lbs_cmd_802_11_sleep_params() defined in file drivers/net/wireless/marvell/libertas/cmd.c returns 0 (indicates success) even when the call to bs_cmd_with_response() (at line 290) fails. When something goes wrong with bs_cmd_with_response(), the parameter sp of function lbs_cmd_802_11_sleep_params() will keep uninitialized. In this case, its caller, function lbs_sleepparams_read() defined in file drivers/net/wireless/marvell/libertas/debugfs.c, will copy uninitialized stack memory to "userbuf", which may result in information leak. I think use "return ret;" instead of "return 0;" in function lbs_cmd_802_11_sleep_params() can avoid the above problem. Codes related to this bug are shown as below. lbs_cmd_802_11_sleep_params @@ drivers/net/wireless/marvell/libertas/cmd.c 269 int lbs_cmd_802_11_sleep_params(struct lbs_private *priv, uint16_t cmd_action, 270 struct sleep_params *sp) 271 { 272 struct cmd_ds_802_11_sleep_params cmd; 273 int ret; 274 275 lbs_deb_enter(LBS_DEB_CMD); 276 277 if (cmd_action == CMD_ACT_GET) { 278 memset(&cmd, 0, sizeof(cmd)); 279 } else { 280 cmd.error = cpu_to_le16(sp->sp_error); 281 cmd.offset = cpu_to_le16(sp->sp_offset); 282 cmd.stabletime = cpu_to_le16(sp->sp_stabletime); 283 cmd.calcontrol = sp->sp_calcontrol; 284 cmd.externalsleepclk = sp->sp_extsleepclk; 285 cmd.reserved = cpu_to_le16(sp->sp_reserved); 286 } 287 cmd.hdr.size = cpu_to_le16(sizeof(cmd)); 288 cmd.action = cpu_to_le16(cmd_action); 289 290 ret = lbs_cmd_with_response(priv, CMD_802_11_SLEEP_PARAMS, &cmd); 291 292 if (!ret) { 293 lbs_deb_cmd("error 0x%x, offset 0x%x, stabletime 0x%x, " 294 "calcontrol 0x%x extsleepclk 0x%x\n", 295 le16_to_cpu(cmd.error), le16_to_cpu(cmd.offset), 296 le16_to_cpu(cmd.stabletime), cmd.calcontrol, 297 cmd.externalsleepclk); 298 299 sp->sp_error = le16_to_cpu(cmd.error); 300 sp->sp_offset = le16_to_cpu(cmd.offset); 301 sp->sp_stabletime = le16_to_cpu(cmd.stabletime); 302 sp->sp_calcontrol = cmd.calcontrol; 303 sp->sp_extsleepclk = cmd.externalsleepclk; 304 sp->sp_reserved = le16_to_cpu(cmd.reserved); 305 } 306 307 lbs_deb_leave_args(LBS_DEB_CMD, "ret %d", ret); 308 return 0; // return ret? 309 } lbs_sleepparams_read @@ drivers/net/wireless/marvell/libertas/debugfs.c 91 static ssize_t lbs_sleepparams_read(struct file *file, char __user *userbuf, 92 size_t count, loff_t *ppos) 93 { 94 struct lbs_private *priv = file->private_data; 95 ssize_t ret; 96 size_t pos = 0; 97 struct sleep_params sp; 98 unsigned long addr = get_zeroed_page(GFP_KERNEL); 99 char *buf = (char *)addr; 100 if (!buf) 101 return -ENOMEM; 102 103 ret = lbs_cmd_802_11_sleep_params(priv, CMD_ACT_GET, &sp); 104 if (ret) 105 goto out_unlock; 106 // sp may be uninitialized 107 pos += snprintf(buf, len, "%d %d %d %d %d %d\n", sp.sp_error, 108 sp.sp_offset, sp.sp_stabletime, 109 sp.sp_calcontrol, sp.sp_extsleepclk, 110 sp.sp_reserved); 111 112 ret = simple_read_from_buffer(userbuf, count, ppos, buf, pos); 113 114 out_unlock: 115 free_page(addr); 116 return ret; 117 } Thanks very much!