1. 30 Nov, 2018 38 commits
  2. 29 Nov, 2018 2 commits
    • Lyude Paul's avatar
      brcmfmac: Fix out of bounds memory access during fw load · b72c51a5
      Lyude Paul authored
      I ended up tracking down some rather nasty issues with f2fs (and other
      filesystem modules) constantly crashing on my kernel down to a
      combination of out of bounds memory accesses, one of which was coming
      from brcmfmac during module load:
      
      [   30.891382] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4356-sdio for chip BCM4356/2
      [   30.894437] ==================================================================
      [   30.901581] BUG: KASAN: global-out-of-bounds in brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
      [   30.909935] Read of size 1 at addr ffff2000024865df by task kworker/6:2/387
      [   30.916805]
      [   30.918261] CPU: 6 PID: 387 Comm: kworker/6:2 Tainted: G           O      4.20.0-rc3Lyude-Test+ #19
      [   30.927251] Hardware name: amlogic khadas-vim2/khadas-vim2, BIOS 2018.07-rc2-armbian 09/11/2018
      [   30.935964] Workqueue: events brcmf_driver_register [brcmfmac]
      [   30.941641] Call trace:
      [   30.944058]  dump_backtrace+0x0/0x3e8
      [   30.947676]  show_stack+0x14/0x20
      [   30.950968]  dump_stack+0x130/0x1c4
      [   30.954406]  print_address_description+0x60/0x25c
      [   30.959066]  kasan_report+0x1b4/0x368
      [   30.962683]  __asan_report_load1_noabort+0x18/0x20
      [   30.967547]  brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
      [   30.967639]  brcmf_sdio_probe+0x163c/0x2050 [brcmfmac]
      [   30.978035]  brcmf_ops_sdio_probe+0x598/0xa08 [brcmfmac]
      [   30.983254]  sdio_bus_probe+0x190/0x398
      [   30.983270]  really_probe+0x2a0/0xa70
      [   30.983296]  driver_probe_device+0x1b4/0x2d8
      [   30.994901]  __driver_attach+0x200/0x280
      [   30.994914]  bus_for_each_dev+0x10c/0x1a8
      [   30.994925]  driver_attach+0x38/0x50
      [   30.994935]  bus_add_driver+0x330/0x608
      [   30.994953]  driver_register+0x140/0x388
      [   31.013965]  sdio_register_driver+0x74/0xa0
      [   31.014076]  brcmf_sdio_register+0x14/0x60 [brcmfmac]
      [   31.023177]  brcmf_driver_register+0xc/0x18 [brcmfmac]
      [   31.023209]  process_one_work+0x654/0x1080
      [   31.032266]  worker_thread+0x4f0/0x1308
      [   31.032286]  kthread+0x2a8/0x320
      [   31.039254]  ret_from_fork+0x10/0x1c
      [   31.039269]
      [   31.044226] The buggy address belongs to the variable:
      [   31.044351]  brcmf_firmware_path+0x11f/0xfffffffffffd3b40 [brcmfmac]
      [   31.055601]
      [   31.057031] Memory state around the buggy address:
      [   31.061800]  ffff200002486480: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
      [   31.068983]  ffff200002486500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      [   31.068993] >ffff200002486580: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
      [   31.068999]                                                     ^
      [   31.069017]  ffff200002486600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      [   31.096521]  ffff200002486680: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
      [   31.096528] ==================================================================
      [   31.096533] Disabling lock debugging due to kernel taint
      
      It appears that when trying to determine the length of the string in the
      alternate firmware path, we make the mistake of not handling the case
      where the firmware path is empty correctly. Since strlen(mp_path) can
      return 0, we'll end up accessing mp_path[-1] when the firmware_path
      isn't provided through the module arguments.
      
      So, fix this by just setting the end char to '\0' by default, and only
      changing it if we have a non-zero length. Additionally, use strnlen()
      with BRCMF_FW_ALTPATH_LEN instead of strlen() just to be extra safe.
      
      Fixes: 2baa3aae ("brcmfmac: introduce brcmf_fw_alloc_request() function")
      Cc: Hante Meuleman <hante.meuleman@broadcom.com>
      Cc: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
      Cc: Franky Lin <franky.lin@broadcom.com>
      Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
      Cc: Kalle Valo <kvalo@codeaurora.org>
      Cc: Arend Van Spriel <arend.vanspriel@broadcom.com>
      Cc: Himanshu Jha <himanshujha199640@gmail.com>
      Cc: Dan Haab <dhaab@luxul.com>
      Cc: Jia-Shyr Chuang <saint.chuang@cypress.com>
      Cc: Ian Molton <ian@mnementh.co.uk>
      Cc: <stable@vger.kernel.org> # v4.17+
      Signed-off-by: default avatarLyude Paul <lyude@redhat.com>
      Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
      b72c51a5
    • Hans de Goede's avatar
      brcmfmac: Call brcmf_dmi_probe before brcmf_of_probe · 554da386
      Hans de Goede authored
      ARM systems with UEFI may have both devicetree (of) and DMI data in this
      case we end up setting brcmf_mp_device.board_type twice.
      
      In this case we should prefer the devicetree data, because:
      1) The devicerree data is more reliable
      2) Some ARM systems (e.g. the Raspberry Pi 3 models) support both UEFI and
         classic uboot booting, the devicetree data is always there, so using it
         makes sure we ask for the same nvram file independent of how we booted.
      
      This commit moves the brcmf_dmi_probe call to before the brcmf_of_probe
      call, so that the latter can override the value of the first if both are
      set.
      
      Fixes: bd1e82bb ("brcmfmac: Set board_type from DMI on x86 based ...")
      Cc: Peter Robinson <pbrobinson@gmail.com>
      Tested-and-reported-by: default avatarPeter Robinson <pbrobinson@gmail.com>
      Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
      Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
      554da386