[Devel,rh7,v3,4/4] ploop: add support for dm-crypted ploops

Submitted by Andrey Ryabinin on Aug. 19, 2016, 1 p.m.

Details

Message ID 1471611621-6838-4-git-send-email-aryabinin@virtuozzo.com
State New
Series "ploop: add support for dm-crypted ploops"
Headers show

Commit Message

Andrey Ryabinin Aug. 19, 2016, 1 p.m.
On dm-crypted ploop fs is mounted not on ploop but on dm-crypt device.
Thus freeze/thaw used by some ploop's ioctl doesn't freeze/thaw filesystem.
To fix that, we store pointer to dm-crypt block device inside ploop_device
struct, and use it to freeze/thaw filesystem.

https://jira.sw.ru/browse/PSBM-50858

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 drivers/block/ploop/dev.c       | 21 ++++++++++++++++++---
 drivers/block/ploop/io_direct.c | 12 ++++++++++++
 drivers/md/dm-crypt.c           |  8 +++++++-
 drivers/md/dm.c                 |  6 ++++++
 drivers/md/dm.h                 |  2 ++
 include/linux/ploop/ploop.h     | 32 ++++++++++++++++++++++++++++++++
 6 files changed, 77 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index 8ed402f..44b5e5e 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -3318,13 +3318,20 @@  void ploop_relax(struct ploop_device * plo)
 }
 
 /* search disk for first partition bdev with mounted fs and freeze it */
-static struct super_block *find_and_freeze_bdev(struct gendisk *disk,
+static struct super_block *find_and_freeze_bdev(struct ploop_device *plo,
 						struct block_device ** bdev_pp)
 {
 	struct super_block  * sb   = NULL;
 	struct block_device * bdev = NULL;
+	struct gendisk *disk = plo->disk;
 	int i;
 
+	bdev = ploop_get_dm_crypt_bdev(plo);
+	if (bdev) {
+		sb = freeze_bdev(bdev);
+		goto out;
+	}
+
 	for (i = 0; i <= (*bdev_pp)->bd_part_count; i++) {
 		bdev = bdget_disk(disk, i);
 		if (!bdev)
@@ -3339,6 +3346,7 @@  static struct super_block *find_and_freeze_bdev(struct gendisk *disk,
 		bdev = NULL;
 	}
 
+out:
 	if (IS_ERR(sb))
 		bdput(bdev);
 	else
@@ -3401,7 +3409,7 @@  static int ploop_snapshot(struct ploop_device * plo, unsigned long arg,
 		/* freeze_bdev() may trigger ploop_bd_full() */
 		plo->maintenance_type = PLOOP_MNTN_SNAPSHOT;
 		mutex_unlock(&plo->ctl_mutex);
-		sb = find_and_freeze_bdev(plo->disk, &bdev);
+		sb = find_and_freeze_bdev(plo, &bdev);
 		mutex_lock(&plo->ctl_mutex);
 		plo->maintenance_type = PLOOP_MNTN_OFF;
 		if (IS_ERR(sb)) {
@@ -4929,9 +4937,15 @@  static int ploop_freeze(struct ploop_device *plo, struct block_device *bdev)
 	if (plo->freeze_state == PLOOP_F_THAWING)
 		return -EBUSY;
 
+	if (plo->dm_crypt_bdev)
+		bdev = plo->dm_crypt_bdev;
+
+	bdgrab(bdev);
 	sb = freeze_bdev(bdev);
-	if (sb && IS_ERR(sb))
+	if (sb && IS_ERR(sb)) {
+		bdput(bdev);
 		return PTR_ERR(sb);
+	}
 
 	plo->frozen_bdev = bdev;
 	plo->freeze_state = PLOOP_F_FROZEN;
@@ -4958,6 +4972,7 @@  static int ploop_thaw(struct ploop_device *plo)
 
 	mutex_unlock(&plo->ctl_mutex);
 	err = thaw_bdev(bdev, sb);
+	bdput(bdev);
 	mutex_lock(&plo->ctl_mutex);
 
 	BUG_ON(plo->freeze_state != PLOOP_F_THAWING);
diff --git a/drivers/block/ploop/io_direct.c b/drivers/block/ploop/io_direct.c
index c12e3c8..6663964 100644
--- a/drivers/block/ploop/io_direct.c
+++ b/drivers/block/ploop/io_direct.c
@@ -871,13 +871,25 @@  static int dio_invalidate_cache(struct address_space * mapping,
 retry:
 	err = invalidate_inode_pages2(mapping);
 	if (err) {
+		struct ploop_device *plo = bdev->bd_disk->private_data;
+		struct block_device *dm_crypt_bdev;
+
 		printk("PLOOP: failed to invalidate page cache %d/%d\n", err, attempt2);
 		if (attempt2)
 			return err;
 		attempt2 = 1;
 
 		mutex_unlock(&mapping->host->i_mutex);
+
+		dm_crypt_bdev = ploop_get_dm_crypt_bdev(plo);
+		if (dm_crypt_bdev)
+			bdev = dm_crypt_bdev;
+		else
+			bdgrab(bdev);
+
 		thaw_bdev(bdev, freeze_bdev(bdev));
+		bdput(bdev);
+
 		mutex_lock(&mapping->host->i_mutex);
 		goto retry;
 	}
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 56ca046..bcdd794 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -35,6 +35,8 @@ 
 
 #define DM_MSG_PREFIX "crypt"
 
+#include <linux/ploop/ploop.h>
+#include "dm.h"
 /*
  * context holding the current state of a multi-part conversion
  */
@@ -1645,8 +1647,10 @@  static void crypt_dtr(struct dm_target *ti)
 	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
 		cc->iv_gen_ops->dtr(cc);
 
-	if (cc->dev)
+	if (cc->dev) {
+		ploop_set_dm_crypt_bdev(cc->dev->bdev, NULL);
 		dm_put_device(ti, cc->dev);
+	}
 
 	kzfree(cc->cipher);
 	kzfree(cc->cipher_string);
@@ -1915,6 +1919,8 @@  static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 
+	ploop_set_dm_crypt_bdev(cc->dev->bdev, dm_md_get_bdev(dm_table_get_md(ti->table)));
+
 	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1) {
 		ti->error = "Invalid device sector";
 		goto bad;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8732079..d9a24c9 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -310,6 +310,12 @@  unsigned dm_get_reserved_rq_based_ios(void)
 }
 EXPORT_SYMBOL_GPL(dm_get_reserved_rq_based_ios);
 
+struct block_device *dm_md_get_bdev(struct mapped_device *md)
+{
+	return md->bdev;
+}
+EXPORT_SYMBOL_GPL(dm_md_get_bdev);
+
 static int __init local_init(void)
 {
 	int r = -ENOMEM;
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 6123c2b..815afa5 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -79,6 +79,8 @@  struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
 
 int dm_queue_merge_is_compulsory(struct request_queue *q);
 
+struct block_device *dm_md_get_bdev(struct mapped_device *md);
+
 void dm_lock_md_type(struct mapped_device *md);
 void dm_unlock_md_type(struct mapped_device *md);
 void dm_set_md_type(struct mapped_device *md, unsigned type);
diff --git a/include/linux/ploop/ploop.h b/include/linux/ploop/ploop.h
index 8262a50..b8c480a 100644
--- a/include/linux/ploop/ploop.h
+++ b/include/linux/ploop/ploop.h
@@ -468,6 +468,7 @@  struct ploop_device
 
 	struct ploop_freeblks_desc *fbd;
 	struct ploop_pushbackup_desc *pbd;
+	struct block_device *dm_crypt_bdev;
 
 	unsigned long		locking_state; /* plo locked by userspace */
 };
@@ -634,6 +635,37 @@  static inline int ploop_req_delay_fua_possible(struct ploop_request *preq)
 	return preq->eng_state == PLOOP_E_DATA_WBI;
 }
 
+static inline void ploop_set_dm_crypt_bdev(struct block_device *ploop_bdev,
+				struct block_device *bdev)
+{
+	if (MAJOR(ploop_bdev->bd_dev) == PLOOP_DEVICE_MAJOR) {
+		struct ploop_device *plo = ploop_bdev->bd_disk->private_data;
+		mutex_lock(&plo->ctl_mutex);
+		plo->dm_crypt_bdev = bdev;
+		mutex_unlock(&plo->ctl_mutex);
+	}
+}
+
+static inline struct block_device *__ploop_get_dm_crypt_bdev(
+	struct ploop_device *plo)
+{
+	if (plo->dm_crypt_bdev)
+		bdgrab(plo->dm_crypt_bdev);
+
+	return plo->dm_crypt_bdev;
+}
+
+static inline struct block_device *ploop_get_dm_crypt_bdev(
+				struct ploop_device *plo)
+{
+	struct block_device *ret;
+
+	mutex_lock(&plo->ctl_mutex);
+	ret = __ploop_get_dm_crypt_bdev(plo);
+	mutex_unlock(&plo->ctl_mutex);
+	return ret;
+}
+
 static inline void ploop_req_set_error(struct ploop_request * preq, int err)
 {
 	if (!preq->error) {

Comments

Maxim Patlasov Aug. 19, 2016, 7:19 p.m.
Andrey,


The patch leads to kernel panic if someone (mistakenly) tries to "thaw" 
ploop device without "freeze" prior. Before the patch the code was 
immune to this because ploop_thaw does nothing if state != FROZEN. The 
rest of the patch looks fine, so


Acked-by: Maxim Patlasov <mpatlasov@virtuozzo.com>


for your patch with the following trivial fix on top:


> @@ -4941,7 +4941,7 @@ static int ploop_freeze(struct ploop_device 
> *plo, struct block_device *bdev)
>  static int ploop_thaw(struct ploop_device *plo)
>  {
>         struct block_device *bdev = plo->frozen_bdev;
> -       struct super_block *sb = bdev->bd_super;
> +       struct super_block *sb = bdev ? bdev->bd_super : NULL;
>         int err;
>
>         if (!test_bit(PLOOP_S_RUNNING, &plo->state))

Thanks,

Maxim





On 08/19/2016 06:00 AM, Andrey Ryabinin wrote:
> On dm-crypted ploop fs is mounted not on ploop but on dm-crypt device.
> Thus freeze/thaw used by some ploop's ioctl doesn't freeze/thaw filesystem.
> To fix that, we store pointer to dm-crypt block device inside ploop_device
> struct, and use it to freeze/thaw filesystem.
>
> https://jira.sw.ru/browse/PSBM-50858
>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>   drivers/block/ploop/dev.c       | 21 ++++++++++++++++++---
>   drivers/block/ploop/io_direct.c | 12 ++++++++++++
>   drivers/md/dm-crypt.c           |  8 +++++++-
>   drivers/md/dm.c                 |  6 ++++++
>   drivers/md/dm.h                 |  2 ++
>   include/linux/ploop/ploop.h     | 32 ++++++++++++++++++++++++++++++++
>   6 files changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 8ed402f..44b5e5e 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -3318,13 +3318,20 @@ void ploop_relax(struct ploop_device * plo)
>   }
>   
>   /* search disk for first partition bdev with mounted fs and freeze it */
> -static struct super_block *find_and_freeze_bdev(struct gendisk *disk,
> +static struct super_block *find_and_freeze_bdev(struct ploop_device *plo,
>   						struct block_device ** bdev_pp)
>   {
>   	struct super_block  * sb   = NULL;
>   	struct block_device * bdev = NULL;
> +	struct gendisk *disk = plo->disk;
>   	int i;
>   
> +	bdev = ploop_get_dm_crypt_bdev(plo);
> +	if (bdev) {
> +		sb = freeze_bdev(bdev);
> +		goto out;
> +	}
> +
>   	for (i = 0; i <= (*bdev_pp)->bd_part_count; i++) {
>   		bdev = bdget_disk(disk, i);
>   		if (!bdev)
> @@ -3339,6 +3346,7 @@ static struct super_block *find_and_freeze_bdev(struct gendisk *disk,
>   		bdev = NULL;
>   	}
>   
> +out:
>   	if (IS_ERR(sb))
>   		bdput(bdev);
>   	else
> @@ -3401,7 +3409,7 @@ static int ploop_snapshot(struct ploop_device * plo, unsigned long arg,
>   		/* freeze_bdev() may trigger ploop_bd_full() */
>   		plo->maintenance_type = PLOOP_MNTN_SNAPSHOT;
>   		mutex_unlock(&plo->ctl_mutex);
> -		sb = find_and_freeze_bdev(plo->disk, &bdev);
> +		sb = find_and_freeze_bdev(plo, &bdev);
>   		mutex_lock(&plo->ctl_mutex);
>   		plo->maintenance_type = PLOOP_MNTN_OFF;
>   		if (IS_ERR(sb)) {
> @@ -4929,9 +4937,15 @@ static int ploop_freeze(struct ploop_device *plo, struct block_device *bdev)
>   	if (plo->freeze_state == PLOOP_F_THAWING)
>   		return -EBUSY;
>   
> +	if (plo->dm_crypt_bdev)
> +		bdev = plo->dm_crypt_bdev;
> +
> +	bdgrab(bdev);
>   	sb = freeze_bdev(bdev);
> -	if (sb && IS_ERR(sb))
> +	if (sb && IS_ERR(sb)) {
> +		bdput(bdev);
>   		return PTR_ERR(sb);
> +	}
>   
>   	plo->frozen_bdev = bdev;
>   	plo->freeze_state = PLOOP_F_FROZEN;
> @@ -4958,6 +4972,7 @@ static int ploop_thaw(struct ploop_device *plo)
>   
>   	mutex_unlock(&plo->ctl_mutex);
>   	err = thaw_bdev(bdev, sb);
> +	bdput(bdev);
>   	mutex_lock(&plo->ctl_mutex);
>   
>   	BUG_ON(plo->freeze_state != PLOOP_F_THAWING);
> diff --git a/drivers/block/ploop/io_direct.c b/drivers/block/ploop/io_direct.c
> index c12e3c8..6663964 100644
> --- a/drivers/block/ploop/io_direct.c
> +++ b/drivers/block/ploop/io_direct.c
> @@ -871,13 +871,25 @@ static int dio_invalidate_cache(struct address_space * mapping,
>   retry:
>   	err = invalidate_inode_pages2(mapping);
>   	if (err) {
> +		struct ploop_device *plo = bdev->bd_disk->private_data;
> +		struct block_device *dm_crypt_bdev;
> +
>   		printk("PLOOP: failed to invalidate page cache %d/%d\n", err, attempt2);
>   		if (attempt2)
>   			return err;
>   		attempt2 = 1;
>   
>   		mutex_unlock(&mapping->host->i_mutex);
> +
> +		dm_crypt_bdev = ploop_get_dm_crypt_bdev(plo);
> +		if (dm_crypt_bdev)
> +			bdev = dm_crypt_bdev;
> +		else
> +			bdgrab(bdev);
> +
>   		thaw_bdev(bdev, freeze_bdev(bdev));
> +		bdput(bdev);
> +
>   		mutex_lock(&mapping->host->i_mutex);
>   		goto retry;
>   	}
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 56ca046..bcdd794 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -35,6 +35,8 @@
>   
>   #define DM_MSG_PREFIX "crypt"
>   
> +#include <linux/ploop/ploop.h>
> +#include "dm.h"
>   /*
>    * context holding the current state of a multi-part conversion
>    */
> @@ -1645,8 +1647,10 @@ static void crypt_dtr(struct dm_target *ti)
>   	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
>   		cc->iv_gen_ops->dtr(cc);
>   
> -	if (cc->dev)
> +	if (cc->dev) {
> +		ploop_set_dm_crypt_bdev(cc->dev->bdev, NULL);
>   		dm_put_device(ti, cc->dev);
> +	}
>   
>   	kzfree(cc->cipher);
>   	kzfree(cc->cipher_string);
> @@ -1915,6 +1919,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>   		goto bad;
>   	}
>   
> +	ploop_set_dm_crypt_bdev(cc->dev->bdev, dm_md_get_bdev(dm_table_get_md(ti->table)));
> +
>   	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1) {
>   		ti->error = "Invalid device sector";
>   		goto bad;
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8732079..d9a24c9 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -310,6 +310,12 @@ unsigned dm_get_reserved_rq_based_ios(void)
>   }
>   EXPORT_SYMBOL_GPL(dm_get_reserved_rq_based_ios);
>   
> +struct block_device *dm_md_get_bdev(struct mapped_device *md)
> +{
> +	return md->bdev;
> +}
> +EXPORT_SYMBOL_GPL(dm_md_get_bdev);
> +
>   static int __init local_init(void)
>   {
>   	int r = -ENOMEM;
> diff --git a/drivers/md/dm.h b/drivers/md/dm.h
> index 6123c2b..815afa5 100644
> --- a/drivers/md/dm.h
> +++ b/drivers/md/dm.h
> @@ -79,6 +79,8 @@ struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
>   
>   int dm_queue_merge_is_compulsory(struct request_queue *q);
>   
> +struct block_device *dm_md_get_bdev(struct mapped_device *md);
> +
>   void dm_lock_md_type(struct mapped_device *md);
>   void dm_unlock_md_type(struct mapped_device *md);
>   void dm_set_md_type(struct mapped_device *md, unsigned type);
> diff --git a/include/linux/ploop/ploop.h b/include/linux/ploop/ploop.h
> index 8262a50..b8c480a 100644
> --- a/include/linux/ploop/ploop.h
> +++ b/include/linux/ploop/ploop.h
> @@ -468,6 +468,7 @@ struct ploop_device
>   
>   	struct ploop_freeblks_desc *fbd;
>   	struct ploop_pushbackup_desc *pbd;
> +	struct block_device *dm_crypt_bdev;
>   
>   	unsigned long		locking_state; /* plo locked by userspace */
>   };
> @@ -634,6 +635,37 @@ static inline int ploop_req_delay_fua_possible(struct ploop_request *preq)
>   	return preq->eng_state == PLOOP_E_DATA_WBI;
>   }
>   
> +static inline void ploop_set_dm_crypt_bdev(struct block_device *ploop_bdev,
> +				struct block_device *bdev)
> +{
> +	if (MAJOR(ploop_bdev->bd_dev) == PLOOP_DEVICE_MAJOR) {
> +		struct ploop_device *plo = ploop_bdev->bd_disk->private_data;
> +		mutex_lock(&plo->ctl_mutex);
> +		plo->dm_crypt_bdev = bdev;
> +		mutex_unlock(&plo->ctl_mutex);
> +	}
> +}
> +
> +static inline struct block_device *__ploop_get_dm_crypt_bdev(
> +	struct ploop_device *plo)
> +{
> +	if (plo->dm_crypt_bdev)
> +		bdgrab(plo->dm_crypt_bdev);
> +
> +	return plo->dm_crypt_bdev;
> +}
> +
> +static inline struct block_device *ploop_get_dm_crypt_bdev(
> +				struct ploop_device *plo)
> +{
> +	struct block_device *ret;
> +
> +	mutex_lock(&plo->ctl_mutex);
> +	ret = __ploop_get_dm_crypt_bdev(plo);
> +	mutex_unlock(&plo->ctl_mutex);
> +	return ret;
> +}
> +
>   static inline void ploop_req_set_error(struct ploop_request * preq, int err)
>   {
>   	if (!preq->error) {