linux-kernel - Re: [PATCH] block: warn once for cloned bio in bio_for_each_segment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+5PVA6Bvm+Lb7CA=bvCwdjhB3JA2KTDz4JCk_FQRipMq03Zkg@mail.gmail.com>
Date:	Mon, 4 Jan 2016 14:21:32 -0500
From:	Josh Boyer <jwboyer@...oraproject.org>
To:	Ming Lei <ming.lei@...onical.com>
Cc:	Jens Axboe <axboe@...nel.dk>,
	"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
	linux-block@...r.kernel.org,
	Kent Overstreet <kent.overstreet@...il.com>,
	"Artem S. Tashkinov" <t.artem@...lcity.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] block: warn once for cloned bio in bio_for_each_segment_all()

On Tue, Dec 22, 2015 at 9:48 PM, Ming Lei <ming.lei@...onical.com> wrote:
> For one cloned bio, kernel shouldn't touch all segments of this
> bio via the helper, because the bvec table is shared among all
> related bios, so warn once for this usage.
>
> This change should catch recent memory corruption issue, which is
> fixed by 23688bf4f830(block: ensure to split after potentially
> bouncing a bio).
>
> Signed-off-by: Ming Lei <ming.lei@...onical.com>
> ---
>  include/linux/bio.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index b9b6e04..b4a298d 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -191,6 +191,7 @@ static inline void *bio_data(struct bio *bio)
>   * before it got to the driver and the driver won't own all of it
>   */
>  #define bio_for_each_segment_all(bvl, bio, i)                          \
> +       WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED));                     \

All this will do is spit out a backtrace.  It doesn't prevent the
corruption from happening, and it requires a user to know to report it
upstream.  That tends to be iffy, as they're more likely to report the
latest issue found in dmesg, which will have occurred after the
corruption.  Is that really the behavior you want?

josh

>         for (i = 0, bvl = (bio)->bi_io_vec; i < (bio)->bi_vcnt; i++, bvl++)
>
>  static inline void bvec_iter_advance(struct bio_vec *bv, struct bvec_iter *iter,
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/