Re: [PATCH V14 04/24] mmc: block: Ensure that debugfs files are removed

Ulf Hansson
2017-11-23T18:13:09Z

On 21 November 2017 at 14:42, Adrian Hunter <adrian.hunter@intel.com> wrote:
> The card is not necessarily being removed, but the debugfs files must be
> removed when the driver is removed, otherwise they will continue to exist
> after unbinding the card from the driver. e.g.
>
> # echo "mmc1:0001" > /sys/bus/mmc/drivers/mmcblk/unbind
> # cat /sys/kernel/debug/mmc1/mmc1\:0001/ext_csd
> [ 173.634584] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
> [ 173.643356] IP: mmc_ext_csd_open+0x5e/0x170
>
> A complication is that the debugfs_root may have already been removed, so
> check for that too.
>
> Fixes: 627c3ccfb46a ("mmc: debugfs: Move block debugfs into block module")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for fixes and added a stable tag!

Kind regards
Uffe

> ---
> drivers/mmc/core/block.c | 44 +++++++++++++++++++++++++++++++++++++-------
> drivers/mmc/core/debugfs.c | 1 +
> 2 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 4a319ddbd956..ccfa98af1dd3 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -122,6 +122,10 @@ struct mmc_blk_data {
> struct device_attribute force_ro;
> struct device_attribute power_ro_lock;
> int area_type;
> +
> + /* debugfs files (only in main mmc_blk_data) */
> + struct dentry *status_dentry;
> + struct dentry *ext_csd_dentry;
> };
>
> /* Device type for RPMB character devices */
> @@ -2653,7 +2657,7 @@ static int mmc_ext_csd_release(struct inode *inode, struct file *file)
> .llseek = default_llseek,
> };
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card)
> +static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> struct dentry *root;
>
> @@ -2663,28 +2667,53 @@ static int mmc_blk_add_debugfs(struct mmc_card *card)
> root = card->debugfs_root;
>
> if (mmc_card_mmc(card) || mmc_card_sd(card)) {
> - if (!debugfs_create_file("status", S_IRUSR, root, card,
> - &mmc_dbg_card_status_fops))
> + md->status_dentry =
> + debugfs_create_file("status", S_IRUSR, root, card,
> + &mmc_dbg_card_status_fops);
> + if (!md->status_dentry)
> return -EIO;
> }
>
> if (mmc_card_mmc(card)) {
> - if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
> - &mmc_dbg_ext_csd_fops))
> + md->ext_csd_dentry =
> + debugfs_create_file("ext_csd", S_IRUSR, root, card,
> + &mmc_dbg_ext_csd_fops);
> + if (!md->ext_csd_dentry)
> return -EIO;
> }
>
> return 0;
> }
>
> +static void mmc_blk_remove_debugfs(struct mmc_card *card,
> + struct mmc_blk_data *md)
> +{
> + if (!card->debugfs_root)
> + return;
> +
> + if (!IS_ERR_OR_NULL(md->status_dentry)) {
> + debugfs_remove(md->status_dentry);
> + md->status_dentry = NULL;
> + }
> +
> + if (!IS_ERR_OR_NULL(md->ext_csd_dentry)) {
> + debugfs_remove(md->ext_csd_dentry);
> + md->ext_csd_dentry = NULL;
> + }
> +}
>
> #else
>
> -static int mmc_blk_add_debugfs(struct mmc_card *card)
> +static int mmc_blk_add_debugfs(struct mmc_card *card, struct mmc_blk_data *md)
> {
> return 0;
> }
>
> +static void mmc_blk_remove_debugfs(struct mmc_card *card,
> + struct mmc_blk_data *md)
> +{
> +}
> +
> #endif /* CONFIG_DEBUG_FS */
>
> static int mmc_blk_probe(struct mmc_card *card)
> @@ -2724,7 +2753,7 @@ static int mmc_blk_probe(struct mmc_card *card)
> }
>
> /* Add two debugfs entries */
> - mmc_blk_add_debugfs(card);
> + mmc_blk_add_debugfs(card, md);
>
> pm_runtime_set_autosuspend_delay(&card->dev, 3000);
> pm_runtime_use_autosuspend(&card->dev);
> @@ -2750,6 +2779,7 @@ static void mmc_blk_remove(struct mmc_card *card)
> {
> struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
>
> + mmc_blk_remove_debugfs(card, md);
> mmc_blk_remove_parts(card, md);
> pm_runtime_get_sync(&card->dev);
> mmc_claim_host(card->host);
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 01e459a34f33..0f4a7d7b2626 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -314,4 +314,5 @@ void mmc_add_card_debugfs(struct mmc_card *card)
> void mmc_remove_card_debugfs(struct mmc_card *card)
> {
> debugfs_remove_recursive(card->debugfs_root);
> + card->debugfs_root = NULL;
> }
> --
> 1.9.1
>