linux-kernel - [GIT PULL REQUEST] one last-minute update for md/raid5

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>] [day] [month] [year] [list]
Message-ID: <20141003153801.7cd83266@notabene.brown>
Date:	Fri, 3 Oct 2014 15:38:01 +1000
From:	NeilBrown <neilb@...e.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	linux RAID <linux-raid@...r.kernel.org>,
	lkml <linux-kernel@...r.kernel.org>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Shaohua Li <shli@...nel.org>,
	Mike Snitzer <snitzer@...hat.com>,
	Heinz Mauelshagen <heinzm@...hat.com>
Subject: [GIT PULL REQUEST] one last-minute update for md/raid5


Hi Linus,
 I hope this is in time for 3.17...
 It turns out that 'discard_zeroes_data' is not a guarantee, and RAID4/5/6
 really depends on it for DISCARD to be handled safely.
 So it is safest to default to disabling DISCARD on these arrays until some
 sort of whitelist arrangement can be found.

Thanks,
NeilBrown


The following changes since commit fe82dcec644244676d55a1384c958d5f67979adb:

  Linux 3.17-rc7 (2014-09-28 14:29:07 -0700)

are available in the git repository at:

  git://neil.brown.name/md tags/md/3.17-final-fix

for you to fetch changes up to 8e0e99ba64c7ba46133a7c8a3e3f7de01f23bd93:

  md/raid5: disable 'DISCARD' by default due to safety concerns. (2014-10-02 13:45:00 +1000)

----------------------------------------------------------------
One fix for raid5 discard issue.

----------------------------------------------------------------
NeilBrown (1):
      md/raid5: disable 'DISCARD' by default due to safety concerns.

 drivers/md/raid5.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
commit 8e0e99ba64c7ba46133a7c8a3e3f7de01f23bd93
Author: NeilBrown <neilb@...e.de>
Date:   Thu Oct 2 13:45:00 2014 +1000

    md/raid5: disable 'DISCARD' by default due to safety concerns.
    
    It has come to my attention (thanks Martin) that 'discard_zeroes_data'
    is only a hint.  Some devices in some cases don't do what it
    says on the label.
    
    The use of DISCARD in RAID5 depends on reads from discarded regions
    being predictably zero.  If a write to a previously discarded region
    performs a read-modify-write cycle it assumes that the parity block
    was consistent with the data blocks.  If all were zero, this would
    be the case.  If some are and some aren't this would not be the case.
    This could lead to data corruption after a device failure when
    data needs to be reconstructed from the parity.
    
    As we cannot trust 'discard_zeroes_data', ignore it by default
    and so disallow DISCARD on all raid4/5/6 arrays.
    
    As many devices are trustworthy, and as there are benefits to using
    DISCARD, add a module parameter to over-ride this caution and cause
    DISCARD to work if discard_zeroes_data is set.
    
    If a site want to enable DISCARD on some arrays but not on others they
    should select DISCARD support at the filesystem level, and set the
    raid456 module parameter.
        raid456.devices_handle_discard_safely=Y
    
    As this is a data-safety issue, I believe this patch is suitable for
    -stable.
    DISCARD support for RAID456 was added in 3.7
    
    Cc: Shaohua Li <shli@...nel.org>
    Cc: "Martin K. Petersen" <martin.petersen@...cle.com>
    Cc: Mike Snitzer <snitzer@...hat.com>
    Cc: Heinz Mauelshagen <heinzm@...hat.com>
    Cc: stable@...r.kernel.org (3.7+)
    Acked-by: Martin K. Petersen <martin.petersen@...cle.com>
    Acked-by: Mike Snitzer <snitzer@...hat.com>
    Fixes: 620125f2bf8ff0c4969b79653b54d7bcc9d40637
    Signed-off-by: NeilBrown <neilb@...e.de>

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 183588b11fc1..9f0fbecd1eb5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -64,6 +64,10 @@
 #define cpu_to_group(cpu) cpu_to_node(cpu)
 #define ANY_GROUP NUMA_NO_NODE
 
+static bool devices_handle_discard_safely = false;
+module_param(devices_handle_discard_safely, bool, 0644);
+MODULE_PARM_DESC(devices_handle_discard_safely,
+		 "Set to Y if all devices in each array reliably return zeroes on reads from discarded regions");
 static struct workqueue_struct *raid5_wq;
 /*
  * Stripe cache
@@ -6208,7 +6212,7 @@ static int run(struct mddev *mddev)
 		mddev->queue->limits.discard_granularity = stripe;
 		/*
 		 * unaligned part of discard request will be ignored, so can't
-		 * guarantee discard_zerors_data
+		 * guarantee discard_zeroes_data
 		 */
 		mddev->queue->limits.discard_zeroes_data = 0;
 
@@ -6233,6 +6237,18 @@ static int run(struct mddev *mddev)
 			    !bdev_get_queue(rdev->bdev)->
 						limits.discard_zeroes_data)
 				discard_supported = false;
+			/* Unfortunately, discard_zeroes_data is not currently
+			 * a guarantee - just a hint.  So we only allow DISCARD
+			 * if the sysadmin has confirmed that only safe devices
+			 * are in use by setting a module parameter.
+			 */
+			if (!devices_handle_discard_safely) {
+				if (discard_supported) {
+					pr_info("md/raid456: discard support disabled due to uncertainty.\n");
+					pr_info("Set raid456.devices_handle_discard_safely=Y to override.\n");
+				}
+				discard_supported = false;
+			}
 		}
 
 		if (discard_supported &&

Content of type "application/pgp-signature" skipped