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

Configure Feed

Select the types of activity you want to include in your feed.

padata: Fix pd UAF once and for all

There is a race condition/UAF in padata_reorder that goes back
to the initial commit. A reference count is taken at the start
of the process in padata_do_parallel, and released at the end in
padata_serial_worker.

This reference count is (and only is) required for padata_replace
to function correctly. If padata_replace is never called then
there is no issue.

In the function padata_reorder which serves as the core of padata,
as soon as padata is added to queue->serial.list, and the associated
spin lock released, that padata may be processed and the reference
count on pd would go away.

Fix this by getting the next padata before the squeue->serial lock
is released.

In order to make this possible, simplify padata_reorder by only
calling it once the next padata arrives.

Fixes: 16295bec6398 ("padata: Generic parallelization/serialization interface")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Herbert Xu 71203f68 73c24371

+37 -98
-3
include/linux/padata.h
··· 91 91 * @cpu: Next CPU to be processed. 92 92 * @cpumask: The cpumasks in use for parallel and serial workers. 93 93 * @reorder_work: work struct for reordering. 94 - * @lock: Reorder lock. 95 94 */ 96 95 struct parallel_data { 97 96 struct padata_shell *ps; ··· 101 102 unsigned int processed; 102 103 int cpu; 103 104 struct padata_cpumask cpumask; 104 - struct work_struct reorder_work; 105 - spinlock_t ____cacheline_aligned lock; 106 105 }; 107 106 108 107 /**
+37 -95
kernel/padata.c
··· 261 261 * be parallel processed by another cpu and is not yet present in 262 262 * the cpu's reorder queue. 263 263 */ 264 - static struct padata_priv *padata_find_next(struct parallel_data *pd, 265 - bool remove_object) 264 + static struct padata_priv *padata_find_next(struct parallel_data *pd, int cpu, 265 + unsigned int processed) 266 266 { 267 267 struct padata_priv *padata; 268 268 struct padata_list *reorder; 269 - int cpu = pd->cpu; 270 269 271 270 reorder = per_cpu_ptr(pd->reorder_list, cpu); 272 271 273 272 spin_lock(&reorder->lock); 274 - if (list_empty(&reorder->list)) { 275 - spin_unlock(&reorder->lock); 276 - return NULL; 277 - } 273 + if (list_empty(&reorder->list)) 274 + goto notfound; 278 275 279 276 padata = list_entry(reorder->list.next, struct padata_priv, list); 280 277 ··· 279 282 * Checks the rare case where two or more parallel jobs have hashed to 280 283 * the same CPU and one of the later ones finishes first. 281 284 */ 282 - if (padata->seq_nr != pd->processed) { 283 - spin_unlock(&reorder->lock); 284 - return NULL; 285 - } 285 + if (padata->seq_nr != processed) 286 + goto notfound; 286 287 287 - if (remove_object) { 288 - list_del_init(&padata->list); 289 - ++pd->processed; 290 - pd->cpu = cpumask_next_wrap(cpu, pd->cpumask.pcpu); 291 - } 292 - 288 + list_del_init(&padata->list); 293 289 spin_unlock(&reorder->lock); 294 290 return padata; 291 + 292 + notfound: 293 + pd->processed = processed; 294 + pd->cpu = cpu; 295 + spin_unlock(&reorder->lock); 296 + return NULL; 295 297 } 296 298 297 - static void padata_reorder(struct parallel_data *pd) 299 + static void padata_reorder(struct padata_priv *padata) 298 300 { 301 + struct parallel_data *pd = padata->pd; 299 302 struct padata_instance *pinst = pd->ps->pinst; 300 - int cb_cpu; 301 - struct padata_priv *padata; 302 - struct padata_serial_queue *squeue; 303 - struct padata_list *reorder; 303 + unsigned int processed; 304 + int cpu; 304 305 305 - /* 306 - * We need to ensure that only one cpu can work on dequeueing of 307 - * the reorder queue the time. Calculating in which percpu reorder 308 - * queue the next object will arrive takes some time. A spinlock 309 - * would be highly contended. Also it is not clear in which order 310 - * the objects arrive to the reorder queues. So a cpu could wait to 311 - * get the lock just to notice that there is nothing to do at the 312 - * moment. Therefore we use a trylock and let the holder of the lock 313 - * care for all the objects enqueued during the holdtime of the lock. 314 - */ 315 - if (!spin_trylock_bh(&pd->lock)) 316 - return; 306 + processed = pd->processed; 307 + cpu = pd->cpu; 317 308 318 - while (1) { 319 - padata = padata_find_next(pd, true); 309 + do { 310 + struct padata_serial_queue *squeue; 311 + int cb_cpu; 320 312 321 - /* 322 - * If the next object that needs serialization is parallel 323 - * processed by another cpu and is still on it's way to the 324 - * cpu's reorder queue, nothing to do for now. 325 - */ 326 - if (!padata) 327 - break; 313 + cpu = cpumask_next_wrap(cpu, pd->cpumask.pcpu); 314 + processed++; 328 315 329 316 cb_cpu = padata->cb_cpu; 330 317 squeue = per_cpu_ptr(pd->squeue, cb_cpu); 331 318 332 319 spin_lock(&squeue->serial.lock); 333 320 list_add_tail(&padata->list, &squeue->serial.list); 334 - spin_unlock(&squeue->serial.lock); 335 - 336 321 queue_work_on(cb_cpu, pinst->serial_wq, &squeue->work); 337 - } 338 322 339 - spin_unlock_bh(&pd->lock); 340 - 341 - /* 342 - * The next object that needs serialization might have arrived to 343 - * the reorder queues in the meantime. 344 - * 345 - * Ensure reorder queue is read after pd->lock is dropped so we see 346 - * new objects from another task in padata_do_serial. Pairs with 347 - * smp_mb in padata_do_serial. 348 - */ 349 - smp_mb(); 350 - 351 - reorder = per_cpu_ptr(pd->reorder_list, pd->cpu); 352 - if (!list_empty(&reorder->list) && padata_find_next(pd, false)) { 353 323 /* 354 - * Other context(eg. the padata_serial_worker) can finish the request. 355 - * To avoid UAF issue, add pd ref here, and put pd ref after reorder_work finish. 324 + * If the next object that needs serialization is parallel 325 + * processed by another cpu and is still on it's way to the 326 + * cpu's reorder queue, end the loop. 356 327 */ 357 - padata_get_pd(pd); 358 - if (!queue_work(pinst->serial_wq, &pd->reorder_work)) 359 - padata_put_pd(pd); 360 - } 361 - } 362 - 363 - static void invoke_padata_reorder(struct work_struct *work) 364 - { 365 - struct parallel_data *pd; 366 - 367 - local_bh_disable(); 368 - pd = container_of(work, struct parallel_data, reorder_work); 369 - padata_reorder(pd); 370 - local_bh_enable(); 371 - /* Pairs with putting the reorder_work in the serial_wq */ 372 - padata_put_pd(pd); 328 + padata = padata_find_next(pd, cpu, processed); 329 + spin_unlock(&squeue->serial.lock); 330 + } while (padata); 373 331 } 374 332 375 333 static void padata_serial_worker(struct work_struct *serial_work) ··· 375 423 struct padata_list *reorder = per_cpu_ptr(pd->reorder_list, hashed_cpu); 376 424 struct padata_priv *cur; 377 425 struct list_head *pos; 426 + bool gotit = true; 378 427 379 428 spin_lock(&reorder->lock); 380 429 /* Sort in ascending order of sequence number. */ ··· 385 432 if ((signed int)(cur->seq_nr - padata->seq_nr) < 0) 386 433 break; 387 434 } 388 - list_add(&padata->list, pos); 435 + if (padata->seq_nr != pd->processed) { 436 + gotit = false; 437 + list_add(&padata->list, pos); 438 + } 389 439 spin_unlock(&reorder->lock); 390 440 391 - /* 392 - * Ensure the addition to the reorder list is ordered correctly 393 - * with the trylock of pd->lock in padata_reorder. Pairs with smp_mb 394 - * in padata_reorder. 395 - */ 396 - smp_mb(); 397 - 398 - padata_reorder(pd); 441 + if (gotit) 442 + padata_reorder(padata); 399 443 } 400 444 EXPORT_SYMBOL(padata_do_serial); 401 445 ··· 582 632 padata_init_squeues(pd); 583 633 pd->seq_nr = -1; 584 634 refcount_set(&pd->refcnt, 1); 585 - spin_lock_init(&pd->lock); 586 635 pd->cpu = cpumask_first(pd->cpumask.pcpu); 587 - INIT_WORK(&pd->reorder_work, invoke_padata_reorder); 588 636 589 637 return pd; 590 638 ··· 1091 1143 1092 1144 if (!ps) 1093 1145 return; 1094 - 1095 - /* 1096 - * Wait for all _do_serial calls to finish to avoid touching 1097 - * freed pd's and ps's. 1098 - */ 1099 - synchronize_rcu(); 1100 1146 1101 1147 mutex_lock(&ps->pinst->lock); 1102 1148 list_del(&ps->list);