On Mon, Jan 13, 2020 at 10:34:10PM -0800, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit: 040a3c33 Merge tag 'iommu-fixes-v5.5-rc5' of git://git.ker.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=120a5d8ee00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=7e89bd00623fe71e > dashboard link: https://syzkaller.appspot.com/bug?extid=002f559bf34c2c7467d0 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > userspace arch: i386 > > Unfortunately, I don't have any reproducer for this crash yet. > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+002f559bf34c2c7467d0@xxxxxxxxxxxxxxxxxxxxxxxxx > > ================================================================== > BUG: KASAN: vmalloc-out-of-bounds in test_bit > include/asm-generic/bitops/instrumented-non-atomic.h:110 [inline] > BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x47f/0x1840 > drivers/acpi/nfit/core.c:495 > Read of size 8 at addr ffffc90002ddbbb8 by task syz-executor.1/5941 > > CPU: 3 PID: 5941 Comm: syz-executor.1 Not tainted 5.5.0-rc5-syzkaller #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x197/0x210 lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0x5/0x30b mm/kasan/report.c:374 > __kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506 > kasan_report+0x12/0x20 mm/kasan/common.c:639 > check_memory_region_inline mm/kasan/generic.c:185 [inline] > check_memory_region+0x134/0x1a0 mm/kasan/generic.c:192 > __kasan_check_read+0x11/0x20 mm/kasan/common.c:95 > test_bit include/asm-generic/bitops/instrumented-non-atomic.h:110 [inline] > acpi_nfit_ctl+0x47f/0x1840 drivers/acpi/nfit/core.c:495 > __nd_ioctl drivers/nvdimm/bus.c:1152 [inline] > nd_ioctl.isra.0+0xfe2/0x1580 drivers/nvdimm/bus.c:1230 drivers/acpi/nfit/core.c 438 int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, 439 unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc) ^^^^^^^^^ "buf" comes from the user. 440 { 441 struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc); 442 struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm); 443 union acpi_object in_obj, in_buf, *out_obj; 444 const struct nd_cmd_desc *desc = NULL; 445 struct device *dev = acpi_desc->dev; 446 struct nd_cmd_pkg *call_pkg = NULL; 447 const char *cmd_name, *dimm_name; 448 unsigned long cmd_mask, dsm_mask; 449 u32 offset, fw_status = 0; 450 acpi_handle handle; 451 const guid_t *guid; 452 int func, rc, i; 453 454 if (cmd_rc) 455 *cmd_rc = -EINVAL; 456 457 if (cmd == ND_CMD_CALL) 458 call_pkg = buf; ^^^^^^^^^^^^^^ 459 func = cmd_to_func(nfit_mem, cmd, call_pkg); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ func is call_pkg->nd_command so it comes from the user. 460 if (func < 0) 461 return func; 462 463 if (nvdimm) { 464 struct acpi_device *adev = nfit_mem->adev; 465 466 if (!adev) 467 return -ENOTTY; 468 469 dimm_name = nvdimm_name(nvdimm); 470 cmd_name = nvdimm_cmd_name(cmd); 471 cmd_mask = nvdimm_cmd_mask(nvdimm); 472 dsm_mask = nfit_mem->dsm_mask; 473 desc = nd_cmd_dimm_desc(cmd); 474 guid = to_nfit_uuid(nfit_mem->family); 475 handle = adev->handle; 476 } else { 477 struct acpi_device *adev = to_acpi_dev(acpi_desc); 478 479 cmd_name = nvdimm_bus_cmd_name(cmd); 480 cmd_mask = nd_desc->cmd_mask; 481 dsm_mask = nd_desc->bus_dsm_mask; 482 desc = nd_cmd_bus_desc(cmd); 483 guid = to_nfit_uuid(NFIT_DEV_BUS); 484 handle = adev->handle; 485 dimm_name = "bus"; 486 } 487 488 if (!desc || (cmd && (desc->out_num + desc->in_num == 0))) 489 return -ENOTTY; 490 491 /* 492 * Check for a valid command. For ND_CMD_CALL, we also have to 493 * make sure that the DSM function is supported. 494 */ 495 if (cmd == ND_CMD_CALL && !test_bit(func, &dsm_mask)) ^^^^ If func is more than sizeof(long) * 8 then this will overflow. The temptation is to add a check on func in cmd_to_func() but capping it at sizeof(long) * 8 feels unnatural and I'm not sure what the max function should be. [Edit. I see below that > 31 is not supported. ] 496 return -ENOTTY; 497 else if (!test_bit(cmd, &cmd_mask)) 498 return -ENOTTY; 499 500 in_obj.type = ACPI_TYPE_PACKAGE; 501 in_obj.package.count = 1; 502 in_obj.package.elements = &in_buf; There is a another problem in acpi_nfit_clear_to_send(). acpi/nfit/core.c 3485 /* prevent security commands from being issued via ioctl */ 3486 static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc, 3487 struct nvdimm *nvdimm, unsigned int cmd, void *buf) 3488 { 3489 struct nd_cmd_pkg *call_pkg = buf; 3490 unsigned int func; 3491 3492 if (nvdimm && cmd == ND_CMD_CALL && 3493 call_pkg->nd_family == NVDIMM_FAMILY_INTEL) { 3494 func = call_pkg->nd_command; 3495 if ((1 << func) & NVDIMM_INTEL_SECURITY_CMDMASK) ^^^^^^^^^ This is undefined if func is greater than 31. 3496 return -EOPNOTSUPP; 3497 } 3498 3499 return __acpi_nfit_clear_to_send(nd_desc, nvdimm, cmd); 3500 } regards, dan carpenter
