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

[Dan Carpenter <error27@gmail.com>]

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.  The atomic_dec_and_test() will
underflow on subsequent iterations through the loop which is normally
harmless.  We may want to convert this to refcount_t so that any
underflows cause a WARN().


    648         }
    649 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter