[PATCH] dm-flakey: Fix memory corruption

[PATCH] dm-flakey: Fix memory corruption

[Kent Overstreet]

So, this code clearly isn't getting tested - at all. Besides this bug,
the parsing for the "corrupt" modes is also broken.

Guys, don't push broken crap, and figure out how to write some tests.

-- >8 --

Cc: dm-devel@lists.linux.dev
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: Mike Snitzer <snitzer@kernel.org>
Cc: Alasdair Kergon <agk@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 drivers/md/dm-flakey.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
index 731467d4ed10..5ceb5d68f65c 100644
--- a/drivers/md/dm-flakey.c
+++ b/drivers/md/dm-flakey.c
@@ -426,7 +430,7 @@ static struct bio *clone_bio(struct dm_target *ti, struct flakey_c *fc, struct b
 	if (!clone)
 		return NULL;
 
-	bio_init(clone, fc->dev->bdev, bio->bi_inline_vecs, nr_iovecs, bio->bi_opf);
+	bio_init(clone, fc->dev->bdev, clone->bi_inline_vecs, nr_iovecs, bio->bi_opf);
 
 	clone->bi_iter.bi_sector = flakey_map_sector(ti, bio->bi_iter.bi_sector);
 	clone->bi_private = bio;
-- 
2.47.2

Re: dm-flakey: Fix memory corruption

[Mike Snitzer]

On Sat, Mar 08, 2025 at 10:50:08AM -0500, Kent Overstreet wrote:
> So, this code clearly isn't getting tested - at all. Besides this bug,
> the parsing for the "corrupt" modes is also broken.
> 
> Guys, don't push broken crap, and figure out how to write some tests.

Thank you sir, may we have another?

Like you never introduced a bug in your life?

Not going to tolerate your entitled primadonna attitude here.  You are
capable of being better, you've chosen not to be on this issue (twice)

> -- >8 --

Also, your patch header is pathetically bad.  But we get it, you're
far too busy to write a proper patch header that explains your change.

Or adds a Fixes tag, e.g.:

Fixes: 1d9a94389853 ("dm flakey: clone pages on write bio before corrupting them")

> Cc: dm-devel@lists.linux.dev
> Cc: Mikulas Patocka <mpatocka@redhat.com>
> Cc: Mike Snitzer <snitzer@kernel.org>
> Cc: Alasdair Kergon <agk@redhat.com>
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> ---
>  drivers/md/dm-flakey.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

And what's with your inaccurate diffstat?

> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 731467d4ed10..5ceb5d68f65c 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -426,7 +430,7 @@ static struct bio *clone_bio(struct dm_target *ti, struct flakey_c *fc, struct b
>  	if (!clone)
>  		return NULL;
>  
> -	bio_init(clone, fc->dev->bdev, bio->bi_inline_vecs, nr_iovecs, bio->bi_opf);
> +	bio_init(clone, fc->dev->bdev, clone->bi_inline_vecs, nr_iovecs, bio->bi_opf);
>  
>  	clone->bi_iter.bi_sector = flakey_map_sector(ti, bio->bi_iter.bi_sector);
>  	clone->bi_private = bio;
> -- 
> 2.47.2
> 

Re: dm-flakey: Fix memory corruption

[Kent Overstreet]

On Sat, Mar 08, 2025 at 01:19:30PM -0500, Mike Snitzer wrote:
> On Sat, Mar 08, 2025 at 10:50:08AM -0500, Kent Overstreet wrote:
> > So, this code clearly isn't getting tested - at all. Besides this bug,
> > the parsing for the "corrupt" modes is also broken.
> > 
> > Guys, don't push broken crap, and figure out how to write some tests.
> 
> Thank you sir, may we have another?
> 
> Like you never introduced a bug in your life?
> 
> Not going to tolerate your entitled primadonna attitude here.  You are
> capable of being better, you've chosen not to be on this issue (twice)

Talking about basic engineering standards is in no way "being a prima
donna". Testing your changes is as basic as it gets, and this code
wasn't tested _at all_.

Re: dm-flakey: Fix memory corruption

[Mike Snitzer]

On Sat, Mar 08, 2025 at 04:50:05PM -0500, Kent Overstreet wrote:
> On Sat, Mar 08, 2025 at 01:19:30PM -0500, Mike Snitzer wrote:
> > On Sat, Mar 08, 2025 at 10:50:08AM -0500, Kent Overstreet wrote:
> > > So, this code clearly isn't getting tested - at all. Besides this bug,
> > > the parsing for the "corrupt" modes is also broken.
> > > 
> > > Guys, don't push broken crap, and figure out how to write some tests.
> > 
> > Thank you sir, may we have another?
> > 
> > Like you never introduced a bug in your life?
> > 
> > Not going to tolerate your entitled primadonna attitude here.  You are
> > capable of being better, you've chosen not to be on this issue (twice)
> 
> Talking about basic engineering standards is in no way "being a prima
> donna". Testing your changes is as basic as it gets, and this code
> wasn't tested _at all_.

"entitled primadonna attitude" was me pulling punches.

I don't disagree that this is a bug that was missed and that proper
testing hasn't been performed (I'd quibble with the no testing part
only because I cannot speak for Mikulas and don't like to assume I
know it all).

But you're missing the very problematic detail: you used a bug in an
optional feature of the test-only dm-flakey target to try to take a
pound of flesh while preaching from your high horse.  That is
unacceptable behaviour that won't be tolerated here.  Be cool and
others will be in return (unless you keep setting fire to bridges).

Fin.

Bring back md-faulty? (was Re: dm-flakey: Fix memory corruption)

[Kent Overstreet]

On Sun, Mar 09, 2025 at 11:44:38AM -0400, Mike Snitzer wrote:
> On Sat, Mar 08, 2025 at 04:50:05PM -0500, Kent Overstreet wrote:
> > On Sat, Mar 08, 2025 at 01:19:30PM -0500, Mike Snitzer wrote:
> > > On Sat, Mar 08, 2025 at 10:50:08AM -0500, Kent Overstreet wrote:
> > > > So, this code clearly isn't getting tested - at all. Besides this bug,
> > > > the parsing for the "corrupt" modes is also broken.
> > > > 
> > > > Guys, don't push broken crap, and figure out how to write some tests.
> > > 
> > > Thank you sir, may we have another?
> > > 
> > > Like you never introduced a bug in your life?
> > > 
> > > Not going to tolerate your entitled primadonna attitude here.  You are
> > > capable of being better, you've chosen not to be on this issue (twice)
> > 
> > Talking about basic engineering standards is in no way "being a prima
> > donna". Testing your changes is as basic as it gets, and this code
> > wasn't tested _at all_.
> 
> "entitled primadonna attitude" was me pulling punches.
> 
> I don't disagree that this is a bug that was missed and that proper
> testing hasn't been performed (I'd quibble with the no testing part
> only because I cannot speak for Mikulas and don't like to assume I
> know it all).
> 
> But you're missing the very problematic detail: you used a bug in an
> optional feature of the test-only dm-flakey target to try to take a
> pound of flesh while preaching from your high horse.  That is
> unacceptable behaviour that won't be tolerated here.  Be cool and
> others will be in return (unless you keep setting fire to bridges).
> 
> Fin.

Mike, saying code needs to be tested is not an "entitled primadonna
attitude". Pushing completely broken code because you made no attempt to
test it and then flipping out when called out over it - that is.

To recap, we're not talking about some obscure corner cases, we're
talking about core documentated functionality in dm-flakey that is
completely broken in ways that show up immediately if you run it - and
there's at least three bugs that I saw; the parsing code, the
clone_bio() memory corruption, and the read side corruption still wasn't
working when I fixed or worked around the other two (write side did).

This isn't your personal project, this is the kernel - there are
standards, and other people depend on your work. dm-flakey is used
heavily by filesystem folks, and additionally, md-faulty was recently
removed because, supposedly, dm-flakey was sufficient.

And that's what I was using before, and it worked fine, so I'm willing
to bring it back and maintain it if dm-flakey can't be relied on.

Re: Bring back md-faulty? (was Re: dm-flakey: Fix memory corruption)

[Mike Snitzer]

On Sun, Mar 09, 2025 at 01:04:22PM -0400, Kent Overstreet wrote:
> On Sun, Mar 09, 2025 at 11:44:38AM -0400, Mike Snitzer wrote:
> > On Sat, Mar 08, 2025 at 04:50:05PM -0500, Kent Overstreet wrote:
> > > On Sat, Mar 08, 2025 at 01:19:30PM -0500, Mike Snitzer wrote:
> > > > On Sat, Mar 08, 2025 at 10:50:08AM -0500, Kent Overstreet wrote:
> > > > > So, this code clearly isn't getting tested - at all. Besides this 
bug,
> > > > > the parsing for the "corrupt" modes is also broken.
> > > > > 
> > > > > Guys, don't push broken crap, and figure out how to write some 
tests.
> > > > 
> > > > Thank you sir, may we have another?
> > > > 
> > > > Like you never introduced a bug in your life?
> > > > 
> > > > Not going to tolerate your entitled primadonna attitude here.  You 
are
> > > > capable of being better, you've chosen not to be on this issue 
(twice)
> > > 
> > > Talking about basic engineering standards is in no way "being a prima
> > > donna". Testing your changes is as basic as it gets, and this code
> > > wasn't tested _at all_.
> > 
> > "entitled primadonna attitude" was me pulling punches.
> > 
> > I don't disagree that this is a bug that was missed and that proper
> > testing hasn't been performed (I'd quibble with the no testing part
> > only because I cannot speak for Mikulas and don't like to assume I
> > know it all).
> > 
> > But you're missing the very problematic detail: you used a bug in an
> > optional feature of the test-only dm-flakey target to try to take a
> > pound of flesh while preaching from your high horse.  That is
> > unacceptable behaviour that won't be tolerated here.  Be cool and
> > others will be in return (unless you keep setting fire to bridges).
> > 
> > Fin.
> 
> Mike, saying code needs to be tested is not an "entitled primadonna
> attitude".

Definition of primadonna: 
"a very temperamental person with an inflated view of their own talent
or importance."

My issue from the start on Friday night (in private) has always been
how holier-than-thou yet abusive you've been since having discovered
this bug in dm-flakey's optional "corrupt_bio_byte" feature.

> Pushing completely broken code because you made no attempt to
> test it and then flipping out when called out over it - that is.

I didn't push commit 1d9a94389853 _because_ I "made no attempt to test
it".  Commit 1d9a94389853 sought to fix a similar but different
corruption in the original "corrupt_bio_byte" implementation (which
proved useful for the specific case it was first developed for).

> To recap, we're not talking about some obscure corner cases, we're
> talking about core documentated functionality in dm-flakey that is
> completely broken in ways that show up immediately if you run it - and
> there's at least three bugs that I saw; the parsing code, the
> clone_bio() memory corruption, and the read side corruption still wasn't
> working when I fixed or worked around the other two (write side did).
> 
> This isn't your personal project, this is the kernel - there are
> standards, and other people depend on your work. dm-flakey is used
> heavily by filesystem folks, and additionally, md-faulty was recently
> removed because, supposedly, dm-flakey was sufficient.
> 
> And that's what I was using before, and it worked fine, so I'm willing
> to bring it back and maintain it if dm-flakey can't be relied on.

This dm-flakey corrupt_bio_byte bug will be fixed upstream this week.

But your recap is devoid of any understanding that my responses to
this dm-devel thread, and your private messages, have primarily taken
issue with how you've chosen to conduct yourself.

I'm not aware of dm-flakey's corrupt_bio_byte being used in upstream
testing frameworks (xfstests only uses flakey's basic capabilities).

Any willingness to elevate dm-flakey's corrupt_bio_byte to wider use
in upstream testing frameworks would have uncovered the need for your
fix.  You were first to notice it.  Rather than be cool about it,
you've been hostile from the start and completely misrepresented the
significance of the bug given the limited scope of who is impacted.

I really am done responding to your escalating campaign of self-owns.