From 8cf7630b29701d364f8df4a50e4f1f5e752b2778 Mon Sep 17 00:00:00 2001 From: Zev Weiss Date: Mon, 11 Mar 2019 23:28:02 -0700 Subject: kernel/sysctl.c: add missing range check in do_proc_dointvec_minmax_conv This bug has apparently existed since the introduction of this function in the pre-git era (4500e91754d3 in Thomas Gleixner's history.git, "[NET]: Add proc_dointvec_userhz_jiffies, use it for proper handling of neighbour sysctls."). As a minimal fix we can simply duplicate the corresponding check in do_proc_dointvec_conv(). Link: http://lkml.kernel.org/r/20190207123426.9202-3-zev@bewilderbeest.net Signed-off-by: Zev Weiss Cc: Brendan Higgins Cc: Iurii Zaikin Cc: Kees Cook Cc: Luis Chamberlain Cc: [2.6.2+] Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/sysctl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 7bb3988425ee..0854197e0e67 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2645,7 +2645,16 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp, { struct do_proc_dointvec_minmax_conv_param *param = data; if (write) { - int val = *negp ? -*lvalp : *lvalp; + int val; + if (*negp) { + if (*lvalp > (unsigned long) INT_MAX + 1) + return -EINVAL; + val = -*lvalp; + } else { + if (*lvalp > (unsigned long) INT_MAX) + return -EINVAL; + val = *lvalp; + } if ((param->min && *param->min > val) || (param->max && *param->max < val)) return -EINVAL; -- cgit v1.2.3 From 2bc4fc60fb3e1a4cae429b889720755ebe9085bb Mon Sep 17 00:00:00 2001 From: Zev Weiss Date: Mon, 11 Mar 2019 23:28:06 -0700 Subject: kernel/sysctl.c: define minmax conv functions in terms of non-minmax versions do_proc_do[u]intvec_minmax_conv() had included open-coded versions of do_proc_do[u]intvec_conv(); the duplication led to buggy inconsistencies (missing range checks). To reduce the likelihood of such problems in the future, we can instead refactor both to be defined in terms of their non-bounded counterparts (plus the added check). Link: http://lkml.kernel.org/r/20190207165138.5oud57vq4ozwb4kh@hatter.bewilderbeest.net Signed-off-by: Zev Weiss Cc: Brendan Higgins Cc: Iurii Zaikin Cc: Kees Cook Cc: Luis Chamberlain Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/sysctl.c | 59 +++++++++++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 33 deletions(-) (limited to 'kernel') diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 0854197e0e67..e5da394d1ca3 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2643,32 +2643,25 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp, int *valp, int write, void *data) { + int tmp, ret; struct do_proc_dointvec_minmax_conv_param *param = data; + /* + * If writing, first do so via a temporary local int so we can + * bounds-check it before touching *valp. + */ + int *ip = write ? &tmp : valp; + + ret = do_proc_dointvec_conv(negp, lvalp, ip, write, data); + if (ret) + return ret; + if (write) { - int val; - if (*negp) { - if (*lvalp > (unsigned long) INT_MAX + 1) - return -EINVAL; - val = -*lvalp; - } else { - if (*lvalp > (unsigned long) INT_MAX) - return -EINVAL; - val = *lvalp; - } - if ((param->min && *param->min > val) || - (param->max && *param->max < val)) + if ((param->min && *param->min > tmp) || + (param->max && *param->max < tmp)) return -EINVAL; - *valp = val; - } else { - int val = *valp; - if (val < 0) { - *negp = true; - *lvalp = -(unsigned long)val; - } else { - *negp = false; - *lvalp = (unsigned long)val; - } + *valp = tmp; } + return 0; } @@ -2717,22 +2710,22 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp, unsigned int *valp, int write, void *data) { + int ret; + unsigned int tmp; struct do_proc_douintvec_minmax_conv_param *param = data; + /* write via temporary local uint for bounds-checking */ + unsigned int *up = write ? &tmp : valp; - if (write) { - unsigned int val = *lvalp; + ret = do_proc_douintvec_conv(lvalp, up, write, data); + if (ret) + return ret; - if (*lvalp > UINT_MAX) - return -EINVAL; - - if ((param->min && *param->min > val) || - (param->max && *param->max < val)) + if (write) { + if ((param->min && *param->min > tmp) || + (param->max && *param->max < tmp)) return -ERANGE; - *valp = val; - } else { - unsigned int val = *valp; - *lvalp = (unsigned long) val; + *valp = tmp; } return 0; -- cgit v1.2.3 From a0bf842e89a3842162aa8514b9bf4611c86fee10 Mon Sep 17 00:00:00 2001 From: Mike Rapoport Date: Mon, 11 Mar 2019 23:30:26 -0700 Subject: swiotlb: add checks for the return value of memblock_alloc*() Add panic() calls if memblock_alloc() returns NULL. The panic() format duplicates the one used by memblock itself and in order to avoid explosion with long parameters list replace open coded allocation size calculations with a local variable. Link: http://lkml.kernel.org/r/1548057848-15136-19-git-send-email-rppt@linux.ibm.com Signed-off-by: Mike Rapoport Cc: Catalin Marinas Cc: Christophe Leroy Cc: Christoph Hellwig Cc: "David S. Miller" Cc: Dennis Zhou Cc: Geert Uytterhoeven Cc: Greentime Hu Cc: Greg Kroah-Hartman Cc: Guan Xuetao Cc: Guo Ren Cc: Guo Ren [c-sky] Cc: Heiko Carstens Cc: Juergen Gross [Xen] Cc: Mark Salter Cc: Matt Turner Cc: Max Filippov Cc: Michael Ellerman Cc: Michal Simek Cc: Paul Burton Cc: Petr Mladek Cc: Richard Weinberger Cc: Rich Felker Cc: Rob Herring Cc: Rob Herring Cc: Russell King Cc: Stafford Horne Cc: Tony Luck Cc: Vineet Gupta Cc: Yoshinori Sato Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/dma/swiotlb.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'kernel') diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 9f5851064aea..dd6a8e2d53a7 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -199,6 +199,7 @@ void __init swiotlb_update_mem_attributes(void) int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) { unsigned long i, bytes; + size_t alloc_size; bytes = nslabs << IO_TLB_SHIFT; @@ -211,12 +212,18 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE * between io_tlb_start and io_tlb_end. */ - io_tlb_list = memblock_alloc( - PAGE_ALIGN(io_tlb_nslabs * sizeof(int)), - PAGE_SIZE); - io_tlb_orig_addr = memblock_alloc( - PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)), - PAGE_SIZE); + alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(int)); + io_tlb_list = memblock_alloc(alloc_size, PAGE_SIZE); + if (!io_tlb_list) + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", + __func__, alloc_size, PAGE_SIZE); + + alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)); + io_tlb_orig_addr = memblock_alloc(alloc_size, PAGE_SIZE); + if (!io_tlb_orig_addr) + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", + __func__, alloc_size, PAGE_SIZE); + for (i = 0; i < io_tlb_nslabs; i++) { io_tlb_list[i] = IO_TLB_SEGSIZE - OFFSET(i, IO_TLB_SEGSIZE); io_tlb_orig_addr[i] = INVALID_PHYS_ADDR; -- cgit v1.2.3 From 8a7f97b902f4fb0d94b355b6b3f1fbd7154cafb9 Mon Sep 17 00:00:00 2001 From: Mike Rapoport Date: Mon, 11 Mar 2019 23:30:31 -0700 Subject: treewide: add checks for the return value of memblock_alloc*() Add check for the return value of memblock_alloc*() functions and call panic() in case of error. The panic message repeats the one used by panicing memblock allocators with adjustment of parameters to include only relevant ones. The replacement was mostly automated with semantic patches like the one below with manual massaging of format strings. @@ expression ptr, size, align; @@ ptr = memblock_alloc(size, align); + if (!ptr) + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__, size, align); [anders.roxell@linaro.org: use '%pa' with 'phys_addr_t' type] Link: http://lkml.kernel.org/r/20190131161046.21886-1-anders.roxell@linaro.org [rppt@linux.ibm.com: fix format strings for panics after memblock_alloc] Link: http://lkml.kernel.org/r/1548950940-15145-1-git-send-email-rppt@linux.ibm.com [rppt@linux.ibm.com: don't panic if the allocation in sparse_buffer_init fails] Link: http://lkml.kernel.org/r/20190131074018.GD28876@rapoport-lnx [akpm@linux-foundation.org: fix xtensa printk warning] Link: http://lkml.kernel.org/r/1548057848-15136-20-git-send-email-rppt@linux.ibm.com Signed-off-by: Mike Rapoport Signed-off-by: Anders Roxell Reviewed-by: Guo Ren [c-sky] Acked-by: Paul Burton [MIPS] Acked-by: Heiko Carstens [s390] Reviewed-by: Juergen Gross [Xen] Reviewed-by: Geert Uytterhoeven [m68k] Acked-by: Max Filippov [xtensa] Cc: Catalin Marinas Cc: Christophe Leroy Cc: Christoph Hellwig Cc: "David S. Miller" Cc: Dennis Zhou Cc: Greentime Hu Cc: Greg Kroah-Hartman Cc: Guan Xuetao Cc: Guo Ren Cc: Mark Salter Cc: Matt Turner Cc: Michael Ellerman Cc: Michal Simek Cc: Petr Mladek Cc: Richard Weinberger Cc: Rich Felker Cc: Rob Herring Cc: Rob Herring Cc: Russell King Cc: Stafford Horne Cc: Tony Luck Cc: Vineet Gupta Cc: Yoshinori Sato Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/dma/swiotlb.c | 4 ++-- kernel/power/snapshot.c | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'kernel') diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index dd6a8e2d53a7..56ac77a80b1f 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -215,13 +215,13 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(int)); io_tlb_list = memblock_alloc(alloc_size, PAGE_SIZE); if (!io_tlb_list) - panic("%s: Failed to allocate %lu bytes align=0x%lx\n", + panic("%s: Failed to allocate %zu bytes align=0x%lx\n", __func__, alloc_size, PAGE_SIZE); alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)); io_tlb_orig_addr = memblock_alloc(alloc_size, PAGE_SIZE); if (!io_tlb_orig_addr) - panic("%s: Failed to allocate %lu bytes align=0x%lx\n", + panic("%s: Failed to allocate %zu bytes align=0x%lx\n", __func__, alloc_size, PAGE_SIZE); for (i = 0; i < io_tlb_nslabs; i++) { diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 4802b039b89f..f08a1e4ee1d4 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -965,6 +965,9 @@ void __init __register_nosave_region(unsigned long start_pfn, /* This allocation cannot fail */ region = memblock_alloc(sizeof(struct nosave_region), SMP_CACHE_BYTES); + if (!region) + panic("%s: Failed to allocate %zu bytes\n", __func__, + sizeof(struct nosave_region)); } region->start_pfn = start_pfn; region->end_pfn = end_pfn; -- cgit v1.2.3 From 26fb3dae0a1ec78bdde4b5b72e0e709503e8c596 Mon Sep 17 00:00:00 2001 From: Mike Rapoport Date: Mon, 11 Mar 2019 23:30:42 -0700 Subject: memblock: drop memblock_alloc_*_nopanic() variants As all the memblock allocation functions return NULL in case of error rather than panic(), the duplicates with _nopanic suffix can be removed. Link: http://lkml.kernel.org/r/1548057848-15136-22-git-send-email-rppt@linux.ibm.com Signed-off-by: Mike Rapoport Acked-by: Greg Kroah-Hartman Reviewed-by: Petr Mladek [printk] Cc: Catalin Marinas Cc: Christophe Leroy Cc: Christoph Hellwig Cc: "David S. Miller" Cc: Dennis Zhou Cc: Geert Uytterhoeven Cc: Greentime Hu Cc: Guan Xuetao Cc: Guo Ren Cc: Guo Ren [c-sky] Cc: Heiko Carstens Cc: Juergen Gross [Xen] Cc: Mark Salter Cc: Matt Turner Cc: Max Filippov Cc: Michael Ellerman Cc: Michal Simek Cc: Paul Burton Cc: Richard Weinberger Cc: Rich Felker Cc: Rob Herring Cc: Rob Herring Cc: Russell King Cc: Stafford Horne Cc: Tony Luck Cc: Vineet Gupta Cc: Yoshinori Sato Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/dma/swiotlb.c | 2 +- kernel/printk/printk.c | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) (limited to 'kernel') diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 56ac77a80b1f..53012db1e53c 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -256,7 +256,7 @@ swiotlb_init(int verbose) bytes = io_tlb_nslabs << IO_TLB_SHIFT; /* Get IO TLB memory from the low pages */ - vstart = memblock_alloc_low_nopanic(PAGE_ALIGN(bytes), PAGE_SIZE); + vstart = memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE); if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose)) return; diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8eee85bb2687..6b7654b8001f 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1143,14 +1143,7 @@ void __init setup_log_buf(int early) if (!new_log_buf_len) return; - if (early) { - new_log_buf = - memblock_alloc(new_log_buf_len, LOG_ALIGN); - } else { - new_log_buf = memblock_alloc_nopanic(new_log_buf_len, - LOG_ALIGN); - } - + new_log_buf = memblock_alloc(new_log_buf_len, LOG_ALIGN); if (unlikely(!new_log_buf)) { pr_err("log_buf_len: %lu bytes not available\n", new_log_buf_len); -- cgit v1.2.3