Re: KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl

From: Dan Carpenter
Date: Tue Jan 14 2020 - 03:43:15 EST


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