[Devel] [PATCH rh7 v2 2/2] ploop: add support for dm-crypted ploops
Maxim Patlasov
mpatlasov at virtuozzo.com
Thu Aug 18 18:26:53 PDT 2016
Andrey,
Simple freeze+thaw leads to kernel panic due to null pointer dereference
in ploop_thaw() because plo->sb may be NULL (if nothing mounted on ploop):
> static int ploop_thaw(struct ploop_device *plo)
> {
> struct super_block *sb = plo->sb;
> struct block_device *bdev = sb->s_bdev;
Again, please, give a patch some simple testing before sending it for
review.
Thanks,
Maxim
On 08/18/2016 09:51 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>
> ---
>
> Changes since v1:
> - fixed deadlock in ploop_freeze()
> - use bdgrab()/bdput() to keep bdev alive
> - use sb->s_bdev in ploop_thaw() instead of plo->dm_crypt_bdev in case
> it changed after freeze
>
> drivers/block/ploop/dev.c | 26 +++++++++++++++++++++-----
> 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, 80 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 453d36e..5271c47 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,18 +4937,25 @@ 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->sb = sb;
> plo->freeze_state = PLOOP_F_FROZEN;
> return 0;
> }
>
> -static int ploop_thaw(struct ploop_device *plo, struct block_device *bdev)
> +static int ploop_thaw(struct ploop_device *plo)
> {
> struct super_block *sb = plo->sb;
> + struct block_device *bdev = sb->s_bdev;
> int err;
>
> if (!test_bit(PLOOP_S_RUNNING, &plo->state))
> @@ -4957,6 +4972,7 @@ static int ploop_thaw(struct ploop_device *plo, struct block_device *bdev)
>
> 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);
> @@ -5086,7 +5102,7 @@ static int ploop_ioctl(struct block_device *bdev, fmode_t fmode, unsigned int cm
> err = ploop_freeze(plo, bdev);
> break;
> case PLOOP_IOC_THAW:
> - err = ploop_thaw(plo, bdev);
> + err = ploop_thaw(plo);
> break;
> default:
> err = -EINVAL;
> 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 b2ef6bd..a92d825 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