Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

exofs: avoid VLA in structures

On the quest to remove all VLAs from the kernel[1] this adjusts several
cases where allocation is made after an array of structures that points
back into the allocation. The allocations are changed to perform
explicit calculations instead of using a Variable Length Array in a
structure.

Additionally, this lets Clang compile this code now, since Clang does
not support VLAIS[2].

[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
[2] https://lkml.kernel.org/r/CA+55aFy6h1c3_rP_bXFedsTXzwW+9Q9MfJaW7GUmMBrAp-fJ9A@mail.gmail.com

[keescook@chromium.org: v2]
Link: http://lkml.kernel.org/r/20180418163546.GA45794@beast
Link: http://lkml.kernel.org/r/20180327203904.GA1151@beast
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: Boaz Harrosh <ooo@electrozaur.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Kees Cook and committed by
Linus Torvalds
20fe9353 86a2bb5a

+114 -66
+48 -34
fs/exofs/ore.c
··· 146 146 struct ore_io_state **pios) 147 147 { 148 148 struct ore_io_state *ios; 149 - struct page **pages; 150 - struct osd_sg_entry *sgilist; 149 + size_t size_ios, size_extra, size_total; 150 + void *ios_extra; 151 + 152 + /* 153 + * The desired layout looks like this, with the extra_allocation 154 + * items pointed at from fields within ios or per_dev: 155 + 151 156 struct __alloc_all_io_state { 152 157 struct ore_io_state ios; 153 158 struct ore_per_dev_state per_dev[numdevs]; 154 159 union { 155 160 struct osd_sg_entry sglist[sgs_per_dev * numdevs]; 156 161 struct page *pages[num_par_pages]; 157 - }; 158 - } *_aios; 162 + } extra_allocation; 163 + } whole_allocation; 159 164 160 - if (likely(sizeof(*_aios) <= PAGE_SIZE)) { 161 - _aios = kzalloc(sizeof(*_aios), GFP_KERNEL); 162 - if (unlikely(!_aios)) { 163 - ORE_DBGMSG("Failed kzalloc bytes=%zd\n", 164 - sizeof(*_aios)); 165 + */ 166 + 167 + /* This should never happen, so abort early if it ever does. */ 168 + if (sgs_per_dev && num_par_pages) { 169 + ORE_DBGMSG("Tried to use both pages and sglist\n"); 170 + *pios = NULL; 171 + return -EINVAL; 172 + } 173 + 174 + if (numdevs > (INT_MAX - sizeof(*ios)) / 175 + sizeof(struct ore_per_dev_state)) 176 + return -ENOMEM; 177 + size_ios = sizeof(*ios) + sizeof(struct ore_per_dev_state) * numdevs; 178 + 179 + if (sgs_per_dev * numdevs > INT_MAX / sizeof(struct osd_sg_entry)) 180 + return -ENOMEM; 181 + if (num_par_pages > INT_MAX / sizeof(struct page *)) 182 + return -ENOMEM; 183 + size_extra = max(sizeof(struct osd_sg_entry) * (sgs_per_dev * numdevs), 184 + sizeof(struct page *) * num_par_pages); 185 + 186 + size_total = size_ios + size_extra; 187 + 188 + if (likely(size_total <= PAGE_SIZE)) { 189 + ios = kzalloc(size_total, GFP_KERNEL); 190 + if (unlikely(!ios)) { 191 + ORE_DBGMSG("Failed kzalloc bytes=%zd\n", size_total); 165 192 *pios = NULL; 166 193 return -ENOMEM; 167 194 } 168 - pages = num_par_pages ? _aios->pages : NULL; 169 - sgilist = sgs_per_dev ? _aios->sglist : NULL; 170 - ios = &_aios->ios; 195 + ios_extra = (char *)ios + size_ios; 171 196 } else { 172 - struct __alloc_small_io_state { 173 - struct ore_io_state ios; 174 - struct ore_per_dev_state per_dev[numdevs]; 175 - } *_aio_small; 176 - union __extra_part { 177 - struct osd_sg_entry sglist[sgs_per_dev * numdevs]; 178 - struct page *pages[num_par_pages]; 179 - } *extra_part; 180 - 181 - _aio_small = kzalloc(sizeof(*_aio_small), GFP_KERNEL); 182 - if (unlikely(!_aio_small)) { 197 + ios = kzalloc(size_ios, GFP_KERNEL); 198 + if (unlikely(!ios)) { 183 199 ORE_DBGMSG("Failed alloc first part bytes=%zd\n", 184 - sizeof(*_aio_small)); 200 + size_ios); 185 201 *pios = NULL; 186 202 return -ENOMEM; 187 203 } 188 - extra_part = kzalloc(sizeof(*extra_part), GFP_KERNEL); 189 - if (unlikely(!extra_part)) { 204 + ios_extra = kzalloc(size_extra, GFP_KERNEL); 205 + if (unlikely(!ios_extra)) { 190 206 ORE_DBGMSG("Failed alloc second part bytes=%zd\n", 191 - sizeof(*extra_part)); 192 - kfree(_aio_small); 207 + size_extra); 208 + kfree(ios); 193 209 *pios = NULL; 194 210 return -ENOMEM; 195 211 } 196 212 197 - pages = num_par_pages ? extra_part->pages : NULL; 198 - sgilist = sgs_per_dev ? extra_part->sglist : NULL; 199 213 /* In this case the per_dev[0].sgilist holds the pointer to 200 214 * be freed 201 215 */ 202 - ios = &_aio_small->ios; 203 216 ios->extra_part_alloc = true; 204 217 } 205 218 206 - if (pages) { 207 - ios->parity_pages = pages; 219 + if (num_par_pages) { 220 + ios->parity_pages = ios_extra; 208 221 ios->max_par_pages = num_par_pages; 209 222 } 210 - if (sgilist) { 223 + if (sgs_per_dev) { 224 + struct osd_sg_entry *sgilist = ios_extra; 211 225 unsigned d; 212 226 213 227 for (d = 0; d < numdevs; ++d) {
+55 -20
fs/exofs/ore_raid.c
··· 71 71 { 72 72 struct __stripe_pages_2d *sp2d; 73 73 unsigned data_devs = group_width - parity; 74 + 75 + /* 76 + * Desired allocation layout is, though when larger than PAGE_SIZE, 77 + * each struct __alloc_1p_arrays is separately allocated: 78 + 74 79 struct _alloc_all_bytes { 75 80 struct __alloc_stripe_pages_2d { 76 81 struct __stripe_pages_2d sp2d; ··· 87 82 char page_is_read[data_devs]; 88 83 } __a1pa[pages_in_unit]; 89 84 } *_aab; 85 + 90 86 struct __alloc_1p_arrays *__a1pa; 91 87 struct __alloc_1p_arrays *__a1pa_end; 92 - const unsigned sizeof__a1pa = sizeof(_aab->__a1pa[0]); 88 + 89 + */ 90 + 91 + char *__a1pa; 92 + char *__a1pa_end; 93 + 94 + const size_t sizeof_stripe_pages_2d = 95 + sizeof(struct __stripe_pages_2d) + 96 + sizeof(struct __1_page_stripe) * pages_in_unit; 97 + const size_t sizeof__a1pa = 98 + ALIGN(sizeof(struct page *) * (2 * group_width) + data_devs, 99 + sizeof(void *)); 100 + const size_t sizeof__a1pa_arrays = sizeof__a1pa * pages_in_unit; 101 + const size_t alloc_total = sizeof_stripe_pages_2d + 102 + sizeof__a1pa_arrays; 103 + 93 104 unsigned num_a1pa, alloc_size, i; 94 105 95 106 /* FIXME: check these numbers in ore_verify_layout */ 96 - BUG_ON(sizeof(_aab->__asp2d) > PAGE_SIZE); 107 + BUG_ON(sizeof_stripe_pages_2d > PAGE_SIZE); 97 108 BUG_ON(sizeof__a1pa > PAGE_SIZE); 98 109 99 - if (sizeof(*_aab) > PAGE_SIZE) { 100 - num_a1pa = (PAGE_SIZE - sizeof(_aab->__asp2d)) / sizeof__a1pa; 101 - alloc_size = sizeof(_aab->__asp2d) + sizeof__a1pa * num_a1pa; 110 + /* 111 + * If alloc_total would be larger than PAGE_SIZE, only allocate 112 + * as many a1pa items as would fill the rest of the page, instead 113 + * of the full pages_in_unit count. 114 + */ 115 + if (alloc_total > PAGE_SIZE) { 116 + num_a1pa = (PAGE_SIZE - sizeof_stripe_pages_2d) / sizeof__a1pa; 117 + alloc_size = sizeof_stripe_pages_2d + sizeof__a1pa * num_a1pa; 102 118 } else { 103 119 num_a1pa = pages_in_unit; 104 - alloc_size = sizeof(*_aab); 120 + alloc_size = alloc_total; 105 121 } 106 122 107 - _aab = kzalloc(alloc_size, GFP_KERNEL); 108 - if (unlikely(!_aab)) { 123 + *psp2d = sp2d = kzalloc(alloc_size, GFP_KERNEL); 124 + if (unlikely(!sp2d)) { 109 125 ORE_DBGMSG("!! Failed to alloc sp2d size=%d\n", alloc_size); 110 126 return -ENOMEM; 111 127 } 128 + /* From here Just call _sp2d_free */ 112 129 113 - sp2d = &_aab->__asp2d.sp2d; 114 - *psp2d = sp2d; /* From here Just call _sp2d_free */ 130 + /* Find start of a1pa area. */ 131 + __a1pa = (char *)sp2d + sizeof_stripe_pages_2d; 132 + /* Find end of the _allocated_ a1pa area. */ 133 + __a1pa_end = __a1pa + alloc_size; 115 134 116 - __a1pa = _aab->__a1pa; 117 - __a1pa_end = __a1pa + num_a1pa; 118 - 135 + /* Allocate additionally needed a1pa items in PAGE_SIZE chunks. */ 119 136 for (i = 0; i < pages_in_unit; ++i) { 137 + struct __1_page_stripe *stripe = &sp2d->_1p_stripes[i]; 138 + 120 139 if (unlikely(__a1pa >= __a1pa_end)) { 121 140 num_a1pa = min_t(unsigned, PAGE_SIZE / sizeof__a1pa, 122 141 pages_in_unit - i); 142 + alloc_size = sizeof__a1pa * num_a1pa; 123 143 124 - __a1pa = kcalloc(num_a1pa, sizeof__a1pa, GFP_KERNEL); 144 + __a1pa = kzalloc(alloc_size, GFP_KERNEL); 125 145 if (unlikely(!__a1pa)) { 126 146 ORE_DBGMSG("!! Failed to _alloc_1p_arrays=%d\n", 127 147 num_a1pa); 128 148 return -ENOMEM; 129 149 } 130 - __a1pa_end = __a1pa + num_a1pa; 150 + __a1pa_end = __a1pa + alloc_size; 131 151 /* First *pages is marked for kfree of the buffer */ 132 - sp2d->_1p_stripes[i].alloc = true; 152 + stripe->alloc = true; 133 153 } 134 154 135 - sp2d->_1p_stripes[i].pages = __a1pa->pages; 136 - sp2d->_1p_stripes[i].scribble = __a1pa->scribble ; 137 - sp2d->_1p_stripes[i].page_is_read = __a1pa->page_is_read; 138 - ++__a1pa; 155 + /* 156 + * Attach all _lp_stripes pointers to the allocation for 157 + * it which was either part of the original PAGE_SIZE 158 + * allocation or the subsequent allocation in this loop. 159 + */ 160 + stripe->pages = (void *)__a1pa; 161 + stripe->scribble = stripe->pages + group_width; 162 + stripe->page_is_read = (char *)stripe->scribble + group_width; 163 + __a1pa += sizeof__a1pa; 139 164 } 140 165 141 166 sp2d->parity = parity;
+11 -12
fs/exofs/super.c
··· 549 549 static int __alloc_dev_table(struct exofs_sb_info *sbi, unsigned numdevs, 550 550 struct exofs_dev **peds) 551 551 { 552 - struct __alloc_ore_devs_and_exofs_devs { 553 - /* Twice bigger table: See exofs_init_comps() and comment at 554 - * exofs_read_lookup_dev_table() 555 - */ 556 - struct ore_dev *oreds[numdevs * 2 - 1]; 557 - struct exofs_dev eds[numdevs]; 558 - } *aoded; 552 + /* Twice bigger table: See exofs_init_comps() and comment at 553 + * exofs_read_lookup_dev_table() 554 + */ 555 + const size_t numores = numdevs * 2 - 1; 559 556 struct exofs_dev *eds; 560 557 unsigned i; 561 558 562 - aoded = kzalloc(sizeof(*aoded), GFP_KERNEL); 563 - if (unlikely(!aoded)) { 559 + sbi->oc.ods = kzalloc(numores * sizeof(struct ore_dev *) + 560 + numdevs * sizeof(struct exofs_dev), GFP_KERNEL); 561 + if (unlikely(!sbi->oc.ods)) { 564 562 EXOFS_ERR("ERROR: failed allocating Device array[%d]\n", 565 563 numdevs); 566 564 return -ENOMEM; 567 565 } 568 566 569 - sbi->oc.ods = aoded->oreds; 570 - *peds = eds = aoded->eds; 567 + /* Start of allocated struct exofs_dev entries */ 568 + *peds = eds = (void *)sbi->oc.ods[numores]; 569 + /* Initialize pointers into struct exofs_dev */ 571 570 for (i = 0; i < numdevs; ++i) 572 - aoded->oreds[i] = &eds[i].ored; 571 + sbi->oc.ods[i] = &eds[i].ored; 573 572 return 0; 574 573 } 575 574