From 5dc164307fd9364320ca8bccb6770e9243742597 Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Thu, 8 Jan 2015 11:42:59 +0000 Subject: [PATCH 1/9] UPSTREAM: arm64: make sys_call_table const As with x86, mark the sys_call_table const such that it will be placed in the .rodata section. This will cause attempts to modify the table (accidental or deliberate) to fail when strict page permissions are in place. In the absence of strict page permissions, there should be no functional change. Signed-off-by: Mark Rutland Acked-by: Will Deacon Signed-off-by: Catalin Marinas Bug: 31660652 Signed-off-by: Jeff Vander Stoep (cherry picked from commit c623b33b4e9599c6ac5076f7db7369eb9869aa04) Change-Id: I8c5aa13b8adfdb71e3c574a59e5bf63f8cee42c5 Signed-off-by: Francisco Franco --- arch/arm64/kernel/sys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c index 3fa98ff14f0e..df20b7918854 100644 --- a/arch/arm64/kernel/sys.c +++ b/arch/arm64/kernel/sys.c @@ -50,7 +50,7 @@ asmlinkage long sys_mmap(unsigned long addr, unsigned long len, * The sys_call_table array must be 4K aligned to be accessible from * kernel/entry.S. */ -void *sys_call_table[__NR_syscalls] __aligned(4096) = { +void * const sys_call_table[__NR_syscalls] __aligned(4096) = { [0 ... __NR_syscalls - 1] = sys_ni_syscall, #include }; From c617cfea3d597e79dde4e61d75145859f50fa919 Mon Sep 17 00:00:00 2001 From: Amit Pundir Date: Tue, 11 Aug 2015 12:34:45 +0530 Subject: [PATCH 2/9] usb: gadget: f_mtp: simplify ptp NULL pointer check Simplify MTP/PTP dev NULL pointer check introduced in Change-Id: Ic44a699d96df2e13467fc081bff88b97dcc5afb2 and restrict it to MTP/PTP function level only. Return ERR_PTR() instead of NULL from mtp_ptp function to skip doing NULL pointer checks all the way up to configfs.c Signed-off-by: Amit Pundir Signed-off-by: Dmitry Shmidt Signed-off-by: engstk --- drivers/usb/gadget/configfs.c | 5 ----- drivers/usb/gadget/function/f_mtp.c | 2 +- drivers/usb/gadget/functions.c | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 77b2ea7431f0..8ea58f08ab68 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -436,11 +436,6 @@ static int config_usb_cfg_link( } f = usb_get_function(fi); - if (f == NULL) { - /* Are we trying to symlink PTP without MTP function? */ - ret = -EINVAL; /* Invalid Configuration */ - goto out; - } if (IS_ERR(f)) { ret = PTR_ERR(f); goto out; diff --git a/drivers/usb/gadget/function/f_mtp.c b/drivers/usb/gadget/function/f_mtp.c index c0b6fa581c05..da021bea2e2f 100644 --- a/drivers/usb/gadget/function/f_mtp.c +++ b/drivers/usb/gadget/function/f_mtp.c @@ -1930,7 +1930,7 @@ struct usb_function *function_alloc_mtp_ptp(struct usb_function_instance *fi, pr_err("\t2: Create MTP function\n"); pr_err("\t3: Create and symlink PTP function" " with a gadget configuration\n"); - return NULL; + return ERR_PTR(-EINVAL); /* Invalid Configuration */ } dev = fi_mtp->dev; diff --git a/drivers/usb/gadget/functions.c b/drivers/usb/gadget/functions.c index 389c1f3d0fee..b13f839e7368 100644 --- a/drivers/usb/gadget/functions.c +++ b/drivers/usb/gadget/functions.c @@ -58,7 +58,7 @@ struct usb_function *usb_get_function(struct usb_function_instance *fi) struct usb_function *f; f = fi->fd->alloc_func(fi); - if ((f == NULL) || IS_ERR(f)) + if (IS_ERR(f)) return f; f->fi = fi; return f; From 394f29fcd850ef6af1f54252bddbf7e15efcbbbe Mon Sep 17 00:00:00 2001 From: Jonathan Hamilton Date: Wed, 21 Sep 2016 12:40:51 -0700 Subject: [PATCH 3/9] ANDROID: video: adf: Avoid directly referencing user pointers Enabling KASAN on a kernel using ADF causes a number of places where user-supplied pointers to ioctls pointers are directly dereferenced without copy_from_user or access_ok. Bug: 31806036 Signed-off-by: Jonathan Hamilton Change-Id: I6e86237aaa6cec0f6e1c385336aefcc5332080ae Signed-off-by: engstk --- drivers/video/adf/adf_fops.c | 73 +++++++++++++++--------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/drivers/video/adf/adf_fops.c b/drivers/video/adf/adf_fops.c index 7fbf33e1cb39..d5c1e5466cff 100644 --- a/drivers/video/adf/adf_fops.c +++ b/drivers/video/adf/adf_fops.c @@ -132,7 +132,7 @@ static int adf_eng_get_data(struct adf_overlay_engine *eng, eng->ops->n_supported_formats)); mutex_lock(&dev->client_lock); - ret = adf_obj_copy_custom_data_to_user(&eng->base, arg->custom_data, + ret = adf_obj_copy_custom_data_to_user(&eng->base, data.custom_data, &data.custom_data_size); mutex_unlock(&dev->client_lock); @@ -144,7 +144,7 @@ static int adf_eng_get_data(struct adf_overlay_engine *eng, goto done; } - if (supported_formats && copy_to_user(arg->supported_formats, + if (supported_formats && copy_to_user(data.supported_formats, supported_formats, n_supported_formats * sizeof(supported_formats[0]))) ret = -EFAULT; @@ -220,56 +220,45 @@ static int adf_device_post_config(struct adf_device *dev, int complete_fence_fd; struct adf_buffer *bufs = NULL; struct adf_interface **intfs = NULL; - size_t n_intfs, n_bufs, i; + struct adf_post_config data; + size_t i; void *custom_data = NULL; - size_t custom_data_size; int ret = 0; + if (copy_from_user(&data, arg, sizeof(data))) + return -EFAULT; + complete_fence_fd = get_unused_fd(); if (complete_fence_fd < 0) return complete_fence_fd; - if (get_user(n_intfs, &arg->n_interfaces)) { - ret = -EFAULT; - goto err_get_user; - } - - if (n_intfs > ADF_MAX_INTERFACES) { + if (data.n_interfaces > ADF_MAX_INTERFACES) { ret = -EINVAL; goto err_get_user; } - if (get_user(n_bufs, &arg->n_bufs)) { - ret = -EFAULT; - goto err_get_user; - } - - if (n_bufs > ADF_MAX_BUFFERS) { + if (data.n_bufs > ADF_MAX_BUFFERS) { ret = -EINVAL; goto err_get_user; } - if (get_user(custom_data_size, &arg->custom_data_size)) { - ret = -EFAULT; - goto err_get_user; - } - - if (custom_data_size > ADF_MAX_CUSTOM_DATA_SIZE) { + if (data.custom_data_size > ADF_MAX_CUSTOM_DATA_SIZE) { ret = -EINVAL; goto err_get_user; } - if (n_intfs) { - intfs = kmalloc(sizeof(intfs[0]) * n_intfs, GFP_KERNEL); + if (data.n_interfaces) { + intfs = kmalloc(sizeof(intfs[0]) * data.n_interfaces, + GFP_KERNEL); if (!intfs) { ret = -ENOMEM; goto err_get_user; } } - for (i = 0; i < n_intfs; i++) { + for (i = 0; i < data.n_interfaces; i++) { u32 intf_id; - if (get_user(intf_id, &arg->interfaces[i])) { + if (get_user(intf_id, &data.interfaces[i])) { ret = -EFAULT; goto err_get_user; } @@ -281,31 +270,31 @@ static int adf_device_post_config(struct adf_device *dev, } } - if (n_bufs) { - bufs = kzalloc(sizeof(bufs[0]) * n_bufs, GFP_KERNEL); + if (data.n_bufs) { + bufs = kzalloc(sizeof(bufs[0]) * data.n_bufs, GFP_KERNEL); if (!bufs) { ret = -ENOMEM; goto err_get_user; } } - for (i = 0; i < n_bufs; i++) { - ret = adf_buffer_import(dev, &arg->bufs[i], &bufs[i]); + for (i = 0; i < data.n_bufs; i++) { + ret = adf_buffer_import(dev, &data.bufs[i], &bufs[i]); if (ret < 0) { memset(&bufs[i], 0, sizeof(bufs[i])); goto err_import; } } - if (custom_data_size) { - custom_data = kzalloc(custom_data_size, GFP_KERNEL); + if (data.custom_data_size) { + custom_data = kzalloc(data.custom_data_size, GFP_KERNEL); if (!custom_data) { ret = -ENOMEM; goto err_import; } - if (copy_from_user(custom_data, arg->custom_data, - custom_data_size)) { + if (copy_from_user(custom_data, data.custom_data, + data.custom_data_size)) { ret = -EFAULT; goto err_import; } @@ -316,8 +305,8 @@ static int adf_device_post_config(struct adf_device *dev, goto err_import; } - complete_fence = adf_device_post_nocopy(dev, intfs, n_intfs, bufs, - n_bufs, custom_data, custom_data_size); + complete_fence = adf_device_post_nocopy(dev, intfs, data.n_interfaces, + bufs, data.n_bufs, custom_data, data.custom_data_size); if (IS_ERR(complete_fence)) { ret = PTR_ERR(complete_fence); goto err_import; @@ -327,7 +316,7 @@ static int adf_device_post_config(struct adf_device *dev, return 0; err_import: - for (i = 0; i < n_bufs; i++) + for (i = 0; i < data.n_bufs; i++) adf_buffer_cleanup(&bufs[i]); err_get_user: @@ -481,19 +470,19 @@ static int adf_device_get_data(struct adf_device *dev, data.n_allowed_attachments); mutex_lock(&dev->client_lock); - ret = adf_obj_copy_custom_data_to_user(&dev->base, arg->custom_data, + ret = adf_obj_copy_custom_data_to_user(&dev->base, data.custom_data, &data.custom_data_size); mutex_unlock(&dev->client_lock); if (ret < 0) goto done; - ret = adf_copy_attachment_list_to_user(arg->attachments, + ret = adf_copy_attachment_list_to_user(data.attachments, data.n_attachments, attach, n_attach); if (ret < 0) goto done; - ret = adf_copy_attachment_list_to_user(arg->allowed_attachments, + ret = adf_copy_attachment_list_to_user(data.allowed_attachments, data.n_allowed_attachments, allowed_attach, n_allowed_attach); if (ret < 0) @@ -592,7 +581,7 @@ static int adf_intf_get_data(struct adf_interface *intf, data.n_available_modes = intf->n_modes; read_unlock_irqrestore(&intf->hotplug_modelist_lock, flags); - if (copy_to_user(arg->available_modes, modelist, modelist_size)) { + if (copy_to_user(data.available_modes, modelist, modelist_size)) { ret = -EFAULT; goto done; } @@ -601,7 +590,7 @@ static int adf_intf_get_data(struct adf_interface *intf, memcpy(&data.current_mode, &intf->current_mode, sizeof(intf->current_mode)); - ret = adf_obj_copy_custom_data_to_user(&intf->base, arg->custom_data, + ret = adf_obj_copy_custom_data_to_user(&intf->base, data.custom_data, &data.custom_data_size); done: mutex_unlock(&dev->client_lock); From 4322405c3223ece089bed347c5b3db8a4d2f0c54 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Thu, 6 Oct 2016 15:58:08 -0700 Subject: [PATCH 4/9] diag: dci: avoid out of bounds memory access Bug: 31864832 Change-Id: I7ef183b218d49ad19e0c33d77a52669306471a49 Signed-off-by: Francisco Franco --- drivers/char/diag/diag_dci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/diag/diag_dci.c b/drivers/char/diag/diag_dci.c index 3bc09927ea90..b4e84e791631 100644 --- a/drivers/char/diag/diag_dci.c +++ b/drivers/char/diag/diag_dci.c @@ -2943,7 +2943,7 @@ int diag_dci_write_proc(uint8_t peripheral, int pkt_type, char *buf, int len) DIAG_LOG(DIAG_DEBUG_DCI, "buf: 0x%p, p: %d, len: %d, f_mask: %d\n", buf, peripheral, len, - driver->feature[peripheral].rcvd_feature_mask); + driver->feature[PERIPHERAL_MODEM].rcvd_feature_mask); return -EINVAL; } From 5dd3b251adb0b6b248ae50c6edec0202a217dab5 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Fri, 7 Oct 2016 10:56:13 -0700 Subject: [PATCH 5/9] binder: blacklist %p kptr_restrict Bug: 31495231 Change-Id: Iebc150f6bc939b56e021424ee44fb30ce8d732fd Signed-off-by: Francisco Franco --- drivers/staging/android/binder.c | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index 54107f75ed07..e5a78b33dd94 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -473,7 +473,7 @@ static void binder_insert_free_buffer(struct binder_proc *proc, new_buffer_size = binder_buffer_size(proc, new_buffer); binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: add free buffer, size %zd, at %p\n", + "%d: add free buffer, size %zd, at %pK\n", proc->pid, new_buffer_size, new_buffer); while (*p) { @@ -551,7 +551,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, struct mm_struct *mm; binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: %s pages %p-%p\n", proc->pid, + "%d: %s pages %pK-%pK\n", proc->pid, allocate ? "allocate" : "free", start, end); if (end <= start) @@ -591,7 +591,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, BUG_ON(*page); *page = alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO); if (*page == NULL) { - pr_err("%d: binder_alloc_buf failed for page at %p\n", + pr_err("%d: binder_alloc_buf failed for page at %pK\n", proc->pid, page_addr); goto err_alloc_page_failed; } @@ -600,7 +600,7 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, flush_cache_vmap((unsigned long)page_addr, (unsigned long)page_addr + PAGE_SIZE); if (ret != 1) { - pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n", + pr_err("%d: binder_alloc_buf failed to map page at %pK in kernel\n", proc->pid, page_addr); goto err_map_kernel_failed; } @@ -709,7 +709,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc, } binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: binder_alloc_buf size %zd got buffer %p size %zd\n", + "%d: binder_alloc_buf size %zd got buffer %pK size %zd\n", proc->pid, size, buffer, buffer_size); has_page_addr = @@ -739,7 +739,7 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc, binder_insert_free_buffer(proc, new_buffer); } binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: binder_alloc_buf size %zd got %p\n", + "%d: binder_alloc_buf size %zd got %pK\n", proc->pid, size, buffer); buffer->data_size = data_size; buffer->offsets_size = offsets_size; @@ -779,7 +779,7 @@ static void binder_delete_free_buffer(struct binder_proc *proc, if (buffer_end_page(prev) == buffer_end_page(buffer)) free_page_end = 0; binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: merge free, buffer %p share page with %p\n", + "%d: merge free, buffer %pK share page with %pK\n", proc->pid, buffer, prev); } @@ -792,14 +792,14 @@ static void binder_delete_free_buffer(struct binder_proc *proc, buffer_start_page(buffer)) free_page_start = 0; binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: merge free, buffer %p share page with %p\n", + "%d: merge free, buffer %pK share page with %pK\n", proc->pid, buffer, prev); } } list_del(&buffer->entry); if (free_page_start || free_page_end) { binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: merge free, buffer %p do not share page%s%s with %p or %p\n", + "%d: merge free, buffer %pK do not share page%s%s with %pK or %pK\n", proc->pid, buffer, free_page_start ? "" : " end", free_page_end ? "" : " start", prev, next); binder_update_page_range(proc, 0, free_page_start ? @@ -820,7 +820,7 @@ static void binder_free_buf(struct binder_proc *proc, ALIGN(buffer->offsets_size, sizeof(void *)); binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%d: binder_free_buf %p size %zd buffer_size %zd\n", + "%d: binder_free_buf %pK size %zd buffer_size %zd\n", proc->pid, buffer, size, buffer_size); BUG_ON(buffer->free); @@ -1246,7 +1246,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, int debug_id = buffer->debug_id; binder_debug(BINDER_DEBUG_TRANSACTION, - "%d buffer release %d, size %zd-%zd, failed at %p\n", + "%d buffer release %d, size %zd-%zd, failed at %pK\n", proc->pid, buffer->debug_id, buffer->data_size, buffer->offsets_size, failed_at); @@ -2092,7 +2092,7 @@ static int binder_thread_write(struct binder_proc *proc, } } binder_debug(BINDER_DEBUG_DEAD_BINDER, - "%d:%d BC_DEAD_BINDER_DONE %016llx found %p\n", + "%d:%d BC_DEAD_BINDER_DONE %016llx found %pK\n", proc->pid, thread->pid, (u64)cookie, death); if (death == NULL) { @@ -2901,7 +2901,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) #ifdef CONFIG_CPU_CACHE_VIPT if (cache_is_vipt_aliasing()) { while (CACHE_COLOUR((vma->vm_start ^ (uint32_t)proc->buffer))) { - pr_info("binder_mmap: %d %lx-%lx maps %p bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer); + pr_info("binder_mmap: %d %lx-%lx maps %pK bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer); vma->vm_start += PAGE_SIZE; } } @@ -2933,7 +2933,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) proc->vma = vma; proc->vma_vm_mm = vma->vm_mm; - /*pr_info("binder_mmap: %d %lx-%lx maps %p\n", + /*pr_info("binder_mmap: %d %lx-%lx maps %pK\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);*/ return 0; @@ -3159,7 +3159,7 @@ static void binder_deferred_release(struct binder_proc *proc) page_addr = proc->buffer + i * PAGE_SIZE; binder_debug(BINDER_DEBUG_BUFFER_ALLOC, - "%s: %d: page %d at %p not freed\n", + "%s: %d: page %d at %pK not freed\n", __func__, proc->pid, i, page_addr); unmap_kernel_range((unsigned long)page_addr, PAGE_SIZE); __free_page(proc->pages[i]); @@ -3238,7 +3238,7 @@ static void print_binder_transaction(struct seq_file *m, const char *prefix, struct binder_transaction *t) { seq_printf(m, - "%s %d: %p from %d:%d to %d:%d code %x flags %x pri %ld r%d", + "%s %d: %pK from %d:%d to %d:%d code %x flags %x pri %ld r%d", prefix, t->debug_id, t, t->from ? t->from->proc->pid : 0, t->from ? t->from->pid : 0, @@ -3252,7 +3252,7 @@ static void print_binder_transaction(struct seq_file *m, const char *prefix, if (t->buffer->target_node) seq_printf(m, " node %d", t->buffer->target_node->debug_id); - seq_printf(m, " size %zd:%zd data %p\n", + seq_printf(m, " size %zd:%zd data %pK\n", t->buffer->data_size, t->buffer->offsets_size, t->buffer->data); } @@ -3260,7 +3260,7 @@ static void print_binder_transaction(struct seq_file *m, const char *prefix, static void print_binder_buffer(struct seq_file *m, const char *prefix, struct binder_buffer *buffer) { - seq_printf(m, "%s %d: %p size %zd:%zd %s\n", + seq_printf(m, "%s %d: %pK size %zd:%zd %s\n", prefix, buffer->debug_id, buffer->data, buffer->data_size, buffer->offsets_size, buffer->transaction ? "active" : "delivered"); From 84b29cd3c3ea9f1d510f505bb7166830ae8c7d9f Mon Sep 17 00:00:00 2001 From: Siqi Lin Date: Tue, 11 Oct 2016 11:50:01 -0700 Subject: [PATCH 6/9] msm: camera: Avoid exposing kernel addresses Usage of %p exposes the kernel addresses, an easy target to kernel write vulnerabilities. With this patch currently %pK prints only Zeros as address. If you need actual address echo 0 > /proc/sys/kernel/kptr_restrict CRs-Fixed: 987011 Change-Id: I6c79f82376936fc646b723872a96a6694fe47cd9 Signed-off-by: Azam Sadiq Pasha Kapatrala Syed Signed-off-by: Siqi Lin Bug: 29464815 Signed-off-by: Francisco Franco --- drivers/media/platform/msm/camera_v2/isp/msm_isp40.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/msm/camera_v2/isp/msm_isp40.c b/drivers/media/platform/msm/camera_v2/isp/msm_isp40.c index fb360501f42a..c0dcec82481b 100644 --- a/drivers/media/platform/msm/camera_v2/isp/msm_isp40.c +++ b/drivers/media/platform/msm/camera_v2/isp/msm_isp40.c @@ -1123,7 +1123,7 @@ static int msm_vfe40_start_fetch_engine(struct vfe_device *vfe_dev, rc = vfe_dev->buf_mgr->ops->get_buf_by_index( vfe_dev->buf_mgr, bufq_handle, fe_cfg->buf_idx, &buf); if (rc < 0 || !buf) { - pr_err("%s: No fetch buffer rc= %d buf= %p\n", + pr_err("%s: No fetch buffer rc= %d buf= %pK\n", __func__, rc, buf); return -EINVAL; } From c3423919718cb8d911ecbbca1432584e17455d99 Mon Sep 17 00:00:00 2001 From: Steve Pfetsch Date: Fri, 14 Oct 2016 15:36:59 -0700 Subject: [PATCH 7/9] drivers: video: Add bounds checking in fb_cmap_to_user Verify that unsigned int value will not become negative before cast to signed int. Bug: 31651010 Change-Id: I548a200f678762042617f11100b6966a405a3920 Signed-off-by: Francisco Franco --- drivers/video/fbdev/core/fbcmap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/fbdev/core/fbcmap.c b/drivers/video/fbdev/core/fbcmap.c index f06b7e52bd83..8bfba4242791 100644 --- a/drivers/video/fbdev/core/fbcmap.c +++ b/drivers/video/fbdev/core/fbcmap.c @@ -190,6 +190,9 @@ int fb_cmap_to_user(const struct fb_cmap *from, struct fb_cmap_user *to) int tooff = 0, fromoff = 0; int size; + if ((int)(to->start) < 0) + return -EINVAL; + if (to->start > from->start) fromoff = to->start - from->start; else From ad5735531a5f30961fb524c341bf0d2ef0f4c00f Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 5 Dec 2016 12:10:29 -0800 Subject: [PATCH 8/9] shmem: fix shm fallocate() list corruption The shmem hole punching with fallocate(FALLOC_FL_PUNCH_HOLE) does not want to race with generating new pages by faulting them in. However, the wait-queue used to delay the page faulting has a serious problem: the wait queue head (in shmem_fallocate()) is allocated on the stack, and the code expects that "wake_up_all()" will make sure that all the queue entries are gone before the stack frame is de-allocated. And that is not at all necessarily the case. Yes, a normal wake-up sequence will remove the wait-queue entry that caused the wakeup (see "autoremove_wake_function()"), but the key wording there is "that caused the wakeup". When there are multiple possible wakeup sources, the wait queue entry may well stay around. And _particularly_ in a page fault path, we may be faulting in new pages from user space while we also have other things going on, and there may well be other pending wakeups. So despite the "wake_up_all()", it's not at all guaranteed that all list entries are removed from the wait queue head on the stack. Fix this by introducing a new wakeup function that removes the list entry unconditionally, even if the target process had already woken up for other reasons. Use that "synchronous" function to set up the waiters in shmem_fault(). This problem has never been seen in the wild afaik, but Dave Jones has reported it on and off while running trinity. We thought we fixed the stack corruption with the blk-mq rq_list locking fix (commit 7fe311302f7d: "blk-mq: update hardware and software queues for sleeping alloc"), but it turns out there was _another_ stack corruptor hiding in the trinity runs. Vegard Nossum (also running trinity) was able to trigger this one fairly consistently, and made us look once again at the shmem code due to the faults often being in that area. Reported-and-tested-by: Vegard Nossum . Reported-by: Dave Jones Signed-off-by: Linus Torvalds Signed-off-by: Francisco Franco --- mm/shmem.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/mm/shmem.c b/mm/shmem.c index 26472ff651b7..33421523ef01 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1275,6 +1275,18 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index, return error; } +/* + * This is like autoremove_wake_function, but it removes the wait queue + * entry unconditionally - even if something else had already woken the + * target. + */ +static int synchronous_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key) +{ + int ret = default_wake_function(wait, mode, sync, key); + list_del_init(&wait->task_list); + return ret; +} + static int shmem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct inode *inode = file_inode(vma->vm_file); @@ -1308,7 +1320,7 @@ static int shmem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) vmf->pgoff >= shmem_falloc->start && vmf->pgoff < shmem_falloc->next) { wait_queue_head_t *shmem_falloc_waitq; - DEFINE_WAIT(shmem_fault_wait); + DEFINE_WAIT_FUNC(shmem_fault_wait, synchronous_wake_function); ret = VM_FAULT_NOPAGE; if ((vmf->flags & FAULT_FLAG_ALLOW_RETRY) && @@ -2092,6 +2104,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset, spin_lock(&inode->i_lock); inode->i_private = NULL; wake_up_all(&shmem_falloc_waitq); + WARN_ON_ONCE(!list_empty(&shmem_falloc_waitq.task_list)); spin_unlock(&inode->i_lock); error = 0; goto out; From 498e0a8565be1e598306581b81ea4f88bf9deaeb Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 27 Oct 2016 09:49:19 -0600 Subject: [PATCH 9/9] blk-mq: update hardware and software queues for sleeping alloc If we end up sleeping due to running out of requests, we should update the hardware and software queues in the map ctx structure. Otherwise we could end up having rq->mq_ctx point to the pre-sleep context, and risk corrupting ctx->rq_list since we'll be grabbing the wrong lock when inserting the request. Reported-by: Dave Jones Reported-by: Chris Mason Tested-by: Chris Mason Fixes: 63581af3f31e ("blk-mq: remove non-blocking pass in blk_mq_map_request") Signed-off-by: Jens Axboe Signed-off-by: Francisco Franco --- block/blk-mq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 691959ecb80f..444e84bea39d 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1127,9 +1127,9 @@ static struct request *blk_mq_map_request(struct request_queue *q, hctx = alloc_data.hctx; } - hctx->queued++; - data->hctx = hctx; - data->ctx = ctx; + data->hctx = alloc_data.hctx; + data->ctx = alloc_data.ctx; + data->hctx->queued++; return rq; }