Re: [bug report] raid5-ppl: PPL support for disks with write-back cache enabled

On Wed, Jun 17, 2026 at 09:59:39AM +0300, Dan Carpenter wrote:
> This code is nine years old, so what I like to do is add it to the KTODO
> in case anyone wants to fix it.
> 
> KTODO: Fix use after free in ppl_do_flush()
> 
> Hello Tomasz Majchrzak,
> 
> Commit 1532d9e87e8b ("raid5-ppl: PPL support for disks with
> write-back cache enabled") from Dec 27, 2017 (linux-next), leads to
> the following Smatch static checker warning:
> 
> 	drivers/md/raid5-ppl.c:646 ppl_do_flush()
> 	warn: 'io' was already freed. (line 647)
> 
> drivers/md/raid5-ppl.c
>     608 static void ppl_do_flush(struct ppl_io_unit *io)
>     609 {
>     610         struct ppl_log *log = io->log;
>     611         struct ppl_conf *ppl_conf = log->ppl_conf;
>     612         struct r5conf *conf = ppl_conf->mddev->private;
>     613         int raid_disks = conf->raid_disks;
>     614         int flushed_disks = 0;
>     615         int i;
>     616 
>     617         atomic_set(&io->pending_flushes, raid_disks);
>     618 
>     619         for_each_set_bit(i, &log->disk_flush_bitmap, raid_disks) {
>     620                 struct md_rdev *rdev;
>     621                 struct block_device *bdev = NULL;
>     622 
>     623                 rdev = conf->disks[i].rdev;
>     624                 if (rdev && !test_bit(Faulty, &rdev->flags))
>     625                         bdev = rdev->bdev;
>     626 
>     627                 if (bdev) {
>     628                         struct bio *bio;
>     629 
>     630                         bio = bio_alloc_bioset(bdev, 0,
>     631                                                REQ_OP_WRITE | REQ_PREFLUSH,
>     632                                                GFP_NOIO, &ppl_conf->flush_bs);
>     633                         bio->bi_private = io;
>     634                         bio->bi_end_io = ppl_flush_endio;
>     635 
>     636                         pr_debug("%s: dev: %ps\n", __func__, bio->bi_bdev);
>     637 
>     638                         submit_bio(bio);
>     639                         flushed_disks++;
>     640                 }
>     641         }
>     642 
>     643         log->disk_flush_bitmap = 0;
>     644 
>     645         for (i = flushed_disks ; i < raid_disks; i++) {
> --> 646                 if (atomic_dec_and_test(&io->pending_flushes))
>     647                         ppl_io_unit_finished(io);
> 
> The ppl_io_unit_finished() function frees "io" so probably there is
> supposed to be a statement after it.

This sentence a word missing.  Probably there is supposed to be a *break*
statement.

regards,
dan carpenter