Re: [PATCH v5] md: protect against crash upon fsync on ro array

[Sebastian Riemer <sebastian.riemer@profitbricks.com>]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

Hi Neil,

please apply this patch! It is correct, now.

It applies to 3.2.y, 3.4.y, 3.7.y and latest 3.8-rc5. All these versions
are affected by this bug.

3.0.y and 2.6.34.y are also affected. Please also find the patches for
these versions attached. I've tested them. They work.

The strange thing is that 2.6.32.y is immune against this bug. So it
must be a regression. The patch restores the same behavior as present in
2.6.32: fsync receives success.

I've tested against the following versions: 3.8-rc5, 3.7.5, 3.4.28,
3.2.37, 3.0.61, 2.6.34.14 and 2.6.32.60.

Cheers,
Sebastian


On 29.01.2013 13:29, Paul Menzel wrote:
>> Any further objection?
> 
> Small typo (occurs) in commit message.


[-- Attachment #2: 0001-md-protect-against-crash-upon-fsync-on-ro-array.txt --]
[-- Type: text/plain, Size: 1504 bytes --]

From adfac4df99edc1a83dced9c732464634d3381a9f Mon Sep 17 00:00:00 2001
From: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Date: Fri, 25 Jan 2013 12:46:59 +0100
Subject: [PATCH v5] md: protect against crash upon fsync on ro array

If an fsync occurs on a read-only array, we need to send a
completion for the IO and may not increment the active IO count.
Otherwise, we hit a bug trace and can't stop the MD array anymore.

By advice of Christoph Hellwig we return success upon a flush
request but we return -EROFS for other writes.
We detect flush requests by checking if the bio has zero sectors.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: NeilBrown <neilb@suse.de>
Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/md/md.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3db3d1b..1e634a6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -307,6 +307,10 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
 		bio_io_error(bio);
 		return;
 	}
+	if (mddev->ro == 1 && unlikely(rw == WRITE)) {
+		bio_endio(bio, bio_sectors(bio) == 0 ? 0 : -EROFS);
+		return;
+	}
 	smp_rmb(); /* Ensure implications of  'active' are visible */
 	rcu_read_lock();
 	if (mddev->suspended) {
-- 
1.7.1

[-- Attachment #3: 0002-md-protect-against-crash-upon-fsync-on-ro-array-3.0.y.txt --]
[-- Type: text/plain, Size: 1521 bytes --]

From 58ecc54ef2ea1692b2a608183901708d3cad5120 Mon Sep 17 00:00:00 2001
From: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Date: Fri, 25 Jan 2013 12:46:59 +0100
Subject: [PATCH 3.0.y] md: protect against crash upon fsync on ro array
Reply-To: linux-raid <linux-raid@vger.kernel.org>

If an fsync occurs on a read-only array, we need to send a
completion for the IO and may not increment the active IO count.
Otherwise, we hit a bug trace and can't stop the MD array anymore.

By advice of Christoph Hellwig we return success upon a flush
request but we return -EROFS for other writes.
We detect flush requests by checking if the bio has zero sectors.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: NeilBrown <neilb@suse.de>
Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/md/md.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 98262e5..4ef75e9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -299,6 +299,10 @@ static int md_make_request(struct request_queue *q, struct bio *bio)
 		bio_io_error(bio);
 		return 0;
 	}
+	if (mddev->ro == 1 && unlikely(rw == WRITE)) {
+		bio_endio(bio, bio_sectors(bio) == 0 ? 0 : -EROFS);
+		return 0;
+	}
 	smp_rmb(); /* Ensure implications of  'active' are visible */
 	rcu_read_lock();
 	if (mddev->suspended) {
-- 
1.7.1


[-- Attachment #4: 0003-md-protect-against-crash-upon-fsync-on-ro-array-2.6.34.y.txt --]
[-- Type: text/plain, Size: 1516 bytes --]

From 58ecc54ef2ea1692b2a608183901708d3cad5120 Mon Sep 17 00:00:00 2001
From: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Date: Fri, 25 Jan 2013 12:46:59 +0100
Subject: [PATCH 2.6.34.y] md: protect against crash upon fsync on ro array
Reply-To: linux-raid <linux-raid@vger.kernel.org>

If an fsync occurs on a read-only array, we need to send a
completion for the IO and may not increment the active IO count.
Otherwise, we hit a bug trace and can't stop the MD array anymore.

By advice of Christoph Hellwig we return success upon a flush
request but we return -EROFS for other writes.
We detect flush requests by checking if the bio has zero sectors.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: NeilBrown <neilb@suse.de>
Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/md/md.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d8e5adc..ba1c0be 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -221,6 +221,10 @@ static int md_make_request(struct request_queue *q, struct bio *bio)
 		bio_io_error(bio);
 		return 0;
 	}
+	if (mddev->ro == 1 && unlikely(bio_data_dir(bio) == WRITE)) {
+		bio_endio(bio, bio_sectors(bio) == 0 ? 0 : -EROFS);
+		return 0;
+	}
 	rcu_read_lock();
 	if (mddev->suspended || mddev->barrier) {
 		DEFINE_WAIT(__wait);

-- 
1.7.1