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

Maxim Patlasov mpatlasov at virtuozzo.com
Fri Aug 19 12:19:57 PDT 2016


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 at 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 at 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) {



More information about the Devel mailing list