All of lore.kernel.org
 help / color / mirror / Atom feed
From: Colin Ian King <colin.king@canonical.com>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@redhat.com>,
	dm-devel@redhat.com, linux-kernel@vger.kernel.org
Subject: re: dm: Forbid requeue of writes to zones
Date: Thu, 3 Jun 2021 23:17:09 +0100	[thread overview]
Message-ID: <7e7530a9-7939-2ad6-bfe1-d3aeeeed1f77@canonical.com> (raw)

Hi,

Static analysis with Coverity on Linux next has found and issue in
drivers/md/dm.c with the following commit:

commit 2c243153d1d4be4e23735cd10984ac17c7a54531
Author: Damien Le Moal <damien.lemoal@wdc.com>
Date:   Wed May 26 06:24:58 2021 +0900

    dm: Forbid requeue of writes to zones

The analysis is as follows:

 828 static void dec_pending(struct dm_io *io, blk_status_t error)
 829 {
 830        unsigned long flags;
 831        blk_status_t io_error;

    1. var_decl: Declaring variable bio without initializer.

 832        struct bio *bio;
 833        struct mapped_device *md = io->md;
 834
 835        /* Push-back supersedes any I/O errors */

    2. Condition !!error, taking true branch.

 836        if (unlikely(error)) {
 837                spin_lock_irqsave(&io->endio_lock, flags);

    3. Condition io->status == 11 /* (blk_status_t)11 */, taking false
branch.

 838                if (!(io->status == BLK_STS_DM_REQUEUE &&
__noflush_suspending(md)))
 839                        io->status = error;
 840                spin_unlock_irqrestore(&io->endio_lock, flags);
 841        }
 842

    4. Condition atomic_dec_and_test(&io->io_count), taking true branch.

 843        if (atomic_dec_and_test(&io->io_count)) {

    5. Condition io->status == 11 /* (blk_status_t)11 */, taking true
branch.

 844                if (io->status == BLK_STS_DM_REQUEUE) {
 845                        /*
 846                         * Target requested pushing back the I/O.
 847                         */
 848                        spin_lock_irqsave(&md->deferred_lock, flags);

    6. Condition __noflush_suspending(md), taking true branch.

 849                        if (__noflush_suspending(md) &&

Uninitialized pointer read
    7. uninit_use_in_call: Using uninitialized value bio when calling
dm_is_zone_write.

 850                            !WARN_ON_ONCE(dm_is_zone_write(md, bio)))
 851                                /* NOTE early return due to
BLK_STS_DM_REQUEUE below */
 852                                bio_list_add_head(&md->deferred,
io->orig_bio);

The pointer bio is not initialized and yet is being used in the call to
function dm_is_zone_write where pointer bio is being accessed. I'm not
sure what the original intent was, but this looks incorrect.

Colin

WARNING: multiple messages have this Message-ID (diff)
From: Colin Ian King <colin.king@canonical.com>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: dm-devel@redhat.com, Mike Snitzer <snitzer@redhat.com>,
	Alasdair Kergon <agk@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [dm-devel] dm: Forbid requeue of writes to zones
Date: Thu, 3 Jun 2021 23:17:09 +0100	[thread overview]
Message-ID: <7e7530a9-7939-2ad6-bfe1-d3aeeeed1f77@canonical.com> (raw)

Hi,

Static analysis with Coverity on Linux next has found and issue in
drivers/md/dm.c with the following commit:

commit 2c243153d1d4be4e23735cd10984ac17c7a54531
Author: Damien Le Moal <damien.lemoal@wdc.com>
Date:   Wed May 26 06:24:58 2021 +0900

    dm: Forbid requeue of writes to zones

The analysis is as follows:

 828 static void dec_pending(struct dm_io *io, blk_status_t error)
 829 {
 830        unsigned long flags;
 831        blk_status_t io_error;

    1. var_decl: Declaring variable bio without initializer.

 832        struct bio *bio;
 833        struct mapped_device *md = io->md;
 834
 835        /* Push-back supersedes any I/O errors */

    2. Condition !!error, taking true branch.

 836        if (unlikely(error)) {
 837                spin_lock_irqsave(&io->endio_lock, flags);

    3. Condition io->status == 11 /* (blk_status_t)11 */, taking false
branch.

 838                if (!(io->status == BLK_STS_DM_REQUEUE &&
__noflush_suspending(md)))
 839                        io->status = error;
 840                spin_unlock_irqrestore(&io->endio_lock, flags);
 841        }
 842

    4. Condition atomic_dec_and_test(&io->io_count), taking true branch.

 843        if (atomic_dec_and_test(&io->io_count)) {

    5. Condition io->status == 11 /* (blk_status_t)11 */, taking true
branch.

 844                if (io->status == BLK_STS_DM_REQUEUE) {
 845                        /*
 846                         * Target requested pushing back the I/O.
 847                         */
 848                        spin_lock_irqsave(&md->deferred_lock, flags);

    6. Condition __noflush_suspending(md), taking true branch.

 849                        if (__noflush_suspending(md) &&

Uninitialized pointer read
    7. uninit_use_in_call: Using uninitialized value bio when calling
dm_is_zone_write.

 850                            !WARN_ON_ONCE(dm_is_zone_write(md, bio)))
 851                                /* NOTE early return due to
BLK_STS_DM_REQUEUE below */
 852                                bio_list_add_head(&md->deferred,
io->orig_bio);

The pointer bio is not initialized and yet is being used in the call to
function dm_is_zone_write where pointer bio is being accessed. I'm not
sure what the original intent was, but this looks incorrect.

Colin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


             reply	other threads:[~2021-06-03 22:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03 22:17 Colin Ian King [this message]
2021-06-03 22:17 ` [dm-devel] dm: Forbid requeue of writes to zones Colin Ian King
2021-06-04  0:08 ` Damien Le Moal
2021-06-04  0:08   ` [dm-devel] " Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e7530a9-7939-2ad6-bfe1-d3aeeeed1f77@canonical.com \
    --to=colin.king@canonical.com \
    --cc=agk@redhat.com \
    --cc=damien.lemoal@wdc.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.