On Fri, Jul 14, 2017 at 9:40 PM, David Sterba <dsterba@xxxxxxxx> wrote: > We've switched to cloned bios in btrfs and hit a nasty bug leading to > corruptions, when cloned bios are iterated by bio_for_each_segment_all. No, you simply can't use bio_for_each_segment_all on cloned bio, and the reason is obviously. > > Report and fix: > https://patchwork.kernel.org/patch/9838535/ > > As a matter of precaution, we've added assertions to btrfs code to catch > the bad usage pattern: > > https://patchwork.kernel.org/patch/9839267/ > > The cloned/bi_vcnt behaviour seems tobe implementation dependent and is > not documented, so this patch at least warns about this one particular > case but this might still be insufficient. > > CC: linux-block@xxxxxxxxxxxxxxx > Signed-off-by: David Sterba <dsterba@xxxxxxxx> > --- > include/linux/bio.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/bio.h b/include/linux/bio.h > index 7b1cf4ba0902..f1ac84edcf39 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -156,6 +156,9 @@ static inline void *bio_data(struct bio *bio) > /* > * drivers should _never_ use the all version - the bio may have been split > * before it got to the driver and the driver won't own all of it > + * > + * Note that cloned bios must not use this as their bi_vcnt may be invalid and > + * this could lead to silent corruptions. > */ > #define bio_for_each_segment_all(bvl, bio, i) \ > for (i = 0, bvl = (bio)->bi_io_vec; i < (bio)->bi_vcnt; i++, bvl++) > -- > 2.13.0 > Maybe we can add a warning here if it is a cloned bio. -- Ming Lei
