[PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Harish Jenny K N]

From: Andrew Gabbasov <andrew_gabbasov@mentor.com>

Since RPMB area is accessible via special ioctl only and boot areas
are unlikely to contain any partitions, exclude them all from listing
in /proc/partitions. This will hide them from various user-level
software (e.g. fdisk), thus avoiding unnecessary access attempts.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
---
 drivers/mmc/core/block.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 20135a5..376e47e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	set_disk_ro(md->disk, md->read_only || default_ro);
 	md->disk->flags = GENHD_FL_EXT_DEVT;
 	if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
-		md->disk->flags |= GENHD_FL_NO_PART_SCAN;
+		md->disk->flags |= GENHD_FL_NO_PART_SCAN
+				   | GENHD_FL_SUPPRESS_PARTITION_INFO;
 
 	/*
 	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
-- 
1.9.1

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Shawn Lin]

在 2018/2/27 19:33, Harish Jenny K N 写道:
> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> 
> Since RPMB area is accessible via special ioctl only and boot areas
> are unlikely to contain any partitions, exclude them all from listing
> in /proc/partitions. This will hide them from various user-level
> software (e.g. fdisk), thus avoiding unnecessary access attempts.
> 
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> ---
>   drivers/mmc/core/block.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 20135a5..376e47e 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>   	set_disk_ro(md->disk, md->read_only || default_ro);
>   	md->disk->flags = GENHD_FL_EXT_DEVT;
>   	if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
> -		md->disk->flags |= GENHD_FL_NO_PART_SCAN;
> +		md->disk->flags |= GENHD_FL_NO_PART_SCAN
> +				   | GENHD_FL_SUPPRESS_PARTITION_INFO;

I would prefer using GENHD_FL_HIDDEN instead of adding all these two
flags.

>   
>   	/*
>   	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
> 


-- 
Best Regards
Shawn Lin

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Alex Lemberg]

While RPMB partition requires special IOCTL

Thanks,
Alex 
On 2/27/18, 1:34 PM, "linux-mmc-owner@vger.kernel.org on behalf of Harish Jenny K N" <linux-mmc-owner@vger.kernel.org on behalf of harish_kandiga@mentor.com> wrote:

    From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
    
    Since RPMB area is accessible via special ioctl only and boot areas
    are unlikely to contain any partitions, exclude them all from listing
    in /proc/partitions. This will hide them from various user-level
    software (e.g. fdisk), thus avoiding unnecessary access attempts.
    
    Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
    Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
    ---
     drivers/mmc/core/block.c | 3 ++-
     1 file changed, 2 insertions(+), 1 deletion(-)
    
    diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
    index 20135a5..376e47e 100644
    --- a/drivers/mmc/core/block.c
    +++ b/drivers/mmc/core/block.c
    @@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
     	set_disk_ro(md->disk, md->read_only || default_ro);
     	md->disk->flags = GENHD_FL_EXT_DEVT;
     	if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
    -		md->disk->flags |= GENHD_FL_NO_PART_SCAN;
    +		md->disk->flags |= GENHD_FL_NO_PART_SCAN
    +				   | GENHD_FL_SUPPRESS_PARTITION_INFO;
     
     	/*
     	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
    -- 
    1.9.1
    
    --
    To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Alex Lemberg]

Hi Andrew,

While RPMB partition requires special IOCTL, the boot partition is only requires "switch partition", which is not unusual operation in eMMC.
Why to prevent users access boot partition?

Thanks,
Alex 

On 2/27/18, 1:34 PM, "linux-mmc-owner@vger.kernel.org on behalf of Harish Jenny K N" <linux-mmc-owner@vger.kernel.org on behalf of harish_kandiga@mentor.com> wrote:

    From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
    
    Since RPMB area is accessible via special ioctl only and boot areas
    are unlikely to contain any partitions, exclude them all from listing
    in /proc/partitions. This will hide them from various user-level
    software (e.g. fdisk), thus avoiding unnecessary access attempts.
    
    Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
    Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
    ---
     drivers/mmc/core/block.c | 3 ++-
     1 file changed, 2 insertions(+), 1 deletion(-)
    
    diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
    index 20135a5..376e47e 100644
    --- a/drivers/mmc/core/block.c
    +++ b/drivers/mmc/core/block.c
    @@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
     	set_disk_ro(md->disk, md->read_only || default_ro);
     	md->disk->flags = GENHD_FL_EXT_DEVT;
     	if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
    -		md->disk->flags |= GENHD_FL_NO_PART_SCAN;
    +		md->disk->flags |= GENHD_FL_NO_PART_SCAN
    +				   | GENHD_FL_SUPPRESS_PARTITION_INFO;
     
     	/*
     	 * As discussed on lkml, GENHD_FL_REMOVABLE should:
    -- 
    1.9.1
    
    --
    To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Harish Jenny K N]

On Tuesday 27 February 2018 08:28 PM, Alex Lemberg wrote:
> Hi Andrew,
>
> While RPMB partition requires special IOCTL, the boot partition is only requires "switch partition", which is not unusual operation in eMMC.
> Why to prevent users access boot partition?
>
> Thanks,
> Alex 

The main intention of the patch was to not have RPMB device in /proc/partitions. Boot partitions are also unlikely to have any partitioning, so it made sense to treat them the same way as RPMB and not list in /proc/partitions.
Now I see that RPMB is converted to a character device and this change may not be required for RPMB.

Correct me if I am wrong. Also any comments are welcome.

Thanks,
Harish Jenny K N

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Linus Walleij]

On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
<harish_kandiga@mentor.com> wrote:

> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>
> Since RPMB area is accessible via special ioctl only and boot areas
> are unlikely to contain any partitions, exclude them all from listing
> in /proc/partitions. This will hide them from various user-level
> software (e.g. fdisk), thus avoiding unnecessary access attempts.
>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>

Makes sense to me, at least it makes the problem smaller not bigger.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

> The main intention of the patch was to not have RPMB device in /proc/partitions.
> Boot partitions are also unlikely to have any partitioning, so it made sense to
> treat them the same way as RPMB and not list in /proc/partitions.
> Now I see that RPMB is converted to a character device and this change
> may not be required for RPMB.

It certainly does not hurt, even if this code path is not called
for the RPMB partition anymore (luckily).

On a general note, I do not feel the work with boot partitions or
the general purpose partitions is finished.

In a lecture I pointed out that:

dd if=/dev/sda of=sda.img

Gives an image of the whole device, and you can restore the
device by doing the inverse of that command.

For MMC devices,

dd if=/dev/mmcblk0 of=mmcblk0.img

does NOT have the same nice property. Instead, since the
special partitions are their own devices and not part of the main
device, they have to be backed up separately.

IMO this breaks the user contract of how a device vs a partition
works.

What we need to do is make the "special partitions" part of the
main block device and stop spawning these special block
devices for each boot partions or general partitions. In addition,
each of these boot partitions or general partitions will get their
own block queue and state container which is not cheap in
runtime memory footprint terms.

So what I want to do (unless someone beats me to it) it to come
up with some way making boot and general partitions part
of the main block device. Possibly the core kernel partitioning
code needs to be augmented. The tentative idea is to just
put the sectors representing these partitions after the main
block device and access them from there, with an offset.

That job isn't entirely trivial though.

Yours,
Linus Walleij

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Harish Jenny K N]

On Friday 02 March 2018 06:23 PM, Linus Walleij wrote:
> On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
> <harish_kandiga@mentor.com> wrote:
>
>> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>>
>> Since RPMB area is accessible via special ioctl only and boot areas
>> are unlikely to contain any partitions, exclude them all from listing
>> in /proc/partitions. This will hide them from various user-level
>> software (e.g. fdisk), thus avoiding unnecessary access attempts.
>>
>> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> Makes sense to me, at least it makes the problem smaller not bigger.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


Any other comments/inputs on this ?


Thanks,
Harish Jenny K N

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Alex Lemberg]

Hi Linus,

On 3/2/18 4:53 AM, Linus Walleij wrote:
> On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
> <harish_kandiga@mentor.com> wrote:
>
>> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>>
>> Since RPMB area is accessible via special ioctl only and boot areas
>> are unlikely to contain any partitions, exclude them all from listing
>> in /proc/partitions. This will hide them from various user-level
>> software (e.g. fdisk), thus avoiding unnecessary access attempts.
>>
>> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> Makes sense to me, at least it makes the problem smaller not bigger.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
>> The main intention of the patch was to not have RPMB device in /proc/partitions.
>> Boot partitions are also unlikely to have any partitioning, so it made sense to
>> treat them the same way as RPMB and not list in /proc/partitions.
>> Now I see that RPMB is converted to a character device and this change
>> may not be required for RPMB.
> It certainly does not hurt, even if this code path is not called
> for the RPMB partition anymore (luckily).
>
> On a general note, I do not feel the work with boot partitions or
> the general purpose partitions is finished.
>
> In a lecture I pointed out that:
>
> dd if=/dev/sda of=sda.img
>
> Gives an image of the whole device, and you can restore the
> device by doing the inverse of that command.
>
> For MMC devices,
>
> dd if=/dev/mmcblk0 of=mmcblk0.img
>
> does NOT have the same nice property. Instead, since the
> special partitions are their own devices and not part of the main
> device, they have to be backed up separately.
>
> IMO this breaks the user contract of how a device vs a partition
> works.
>
> What we need to do is make the "special partitions" part of the
> main block device and stop spawning these special block
> devices for each boot partions or general partitions. In addition,
> each of these boot partitions or general partitions will get their
> own block queue and state container which is not cheap in
> runtime memory footprint terms.
>
> So what I want to do (unless someone beats me to it) it to come
> up with some way making boot and general partitions part
> of the main block device. Possibly the core kernel partitioning
> code needs to be augmented. The tentative idea is to just
> put the sectors representing these partitions after the main
> block device and access them from there, with an offset.
I don't think that hiding the Boot and RPMB will resolve the problem
described above.
Boot partition (same as RPMB) in eMMC device is a separate
"physical" partition.
It has its own logical address range and different from general
partition characteristics.
>From the protocol - the access to this partition it requires switch
partition command. It can be accesses during the boot sequence
before the general/userdata partition is mounted.
>From the device side - it can be managed in totally different manner
(SLC vs. MLC blocks, etc.)
I think it completely makes sense to allow access to Boot partition from the
user space. For example - to allow R/W the boot image.

AFAIK, in case of SCSI/UFS devices - Boot LUN's are represented as
separate block
device partitions (/dev/sdb, dev/sdc...).
Shouldn't we have the same for eMMC?

> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Linus Walleij]

On Thu, Mar 8, 2018 at 9:36 PM, Alex Lemberg <Alex.Lemberg@wdc.com> wrote:
> On 3/2/18 4:53 AM, Linus Walleij wrote:

>> What we need to do is make the "special partitions" part of the
>> main block device and stop spawning these special block
>> devices for each boot partions or general partitions. In addition,
>> each of these boot partitions or general partitions will get their
>> own block queue and state container which is not cheap in
>> runtime memory footprint terms.
>>
>> So what I want to do (unless someone beats me to it) it to come
>> up with some way making boot and general partitions part
>> of the main block device. Possibly the core kernel partitioning
>> code needs to be augmented. The tentative idea is to just
>> put the sectors representing these partitions after the main
>> block device and access them from there, with an offset.
>
> I don't think that hiding the Boot and RPMB will resolve the problem
> described above.

Me neither. I'm just trying to discuss the problem lurking behind
these partitions.

> Boot partition (same as RPMB) in eMMC device is a separate
> "physical" partition.
> It has its own logical address range and different from general
> partition characteristics.

Yep.

> From the protocol - the access to this partition it requires switch
> partition command.

Yeah I saw that as well... it's a bit funny.

> From the device side - it can be managed in totally different manner
> (SLC vs. MLC blocks, etc.)
> I think it completely makes sense to allow access to Boot partition from the
> user space. For example - to allow R/W the boot image.

But this patch doesn't hide the partition from userspace does it?

They will still appear in /dev/mmcblk0boot1 etc.

Just not reported as "real" partitions in /proc/partitions.

Or do I misunderstand it?

> AFAIK, in case of SCSI/UFS devices - Boot LUN's are represented as
> separate block
> device partitions (/dev/sdb, dev/sdc...).
> Shouldn't we have the same for eMMC?

I think we should, but we have the problem with the boot partitions
and general partitions that they do not work anything like SCSCI
LUNs.

On a SCSI device dd from the whole device will copy all data on
the device, the partition table and contents of all partitions.

For say /dev/mmcblk0 this is not true of the device contains
boot or general partitions, those other partitions will not be
copied.

Yours,
Linus Walleij

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Harish Jenny K N]

On Saturday 10 March 2018 05:29 PM, Linus Walleij wrote:
>
> But this patch doesn't hide the partition from userspace does it?
>
> They will still appear in /dev/mmcblk0boot1 etc.
>
> Just not reported as "real" partitions in /proc/partitions.
>
> Or do I misunderstand it?
>
>

You are correct. This patch does not hide partition from userspace.
They will still appear in /dev/. But not reported as "real" partitions in /proc/partiotions.

Thanks,
Harish Jenny K N

Re: [PATCH] mmc: card: Don't show eMMC RPMB and BOOT areas in /proc/partitions

[Ulf Hansson]

On 27 February 2018 at 12:33, Harish Jenny K N
<harish_kandiga@mentor.com> wrote:
> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>
> Since RPMB area is accessible via special ioctl only and boot areas
> are unlikely to contain any partitions, exclude them all from listing
> in /proc/partitions. This will hide them from various user-level
> software (e.g. fdisk), thus avoiding unnecessary access attempts.
>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>

Queued up for 3.18, thanks!

Kind regards
Uffe

> ---
>  drivers/mmc/core/block.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 20135a5..376e47e 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2341,7 +2341,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>         set_disk_ro(md->disk, md->read_only || default_ro);
>         md->disk->flags = GENHD_FL_EXT_DEVT;
>         if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
> -               md->disk->flags |= GENHD_FL_NO_PART_SCAN;
> +               md->disk->flags |= GENHD_FL_NO_PART_SCAN
> +                                  | GENHD_FL_SUPPRESS_PARTITION_INFO;
>
>         /*
>          * As discussed on lkml, GENHD_FL_REMOVABLE should:
> --
> 1.9.1
>