2018-02-27 11:35:28

by Harish Jenny K N

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

From: Andrew Gabbasov <[email protected]>

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 <[email protected]>
Signed-off-by: Harish Jenny K N <[email protected]>
---
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



2018-02-27 13:52:16

by Shawn Lin

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

?? 2018/2/27 19:33, Harish Jenny K N д??:
> From: Andrew Gabbasov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Harish Jenny K N <[email protected]>
> ---
> 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


2018-02-27 14:58:52

by Alex Lemberg

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

While RPMB partition requires special IOCTL

Thanks,
Alex
On 2/27/18, 1:34 PM, "[email protected] on behalf of Harish Jenny K N" <[email protected] on behalf of [email protected]> wrote:

From: Andrew Gabbasov <[email protected]>

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 <[email protected]>
Signed-off-by: Harish Jenny K N <[email protected]>
---
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html


2018-02-27 14:59:56

by Alex Lemberg

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

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, "[email protected] on behalf of Harish Jenny K N" <[email protected] on behalf of [email protected]> wrote:

From: Andrew Gabbasov <[email protected]>

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 <[email protected]>
Signed-off-by: Harish Jenny K N <[email protected]>
---
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html


2018-03-01 05:36:59

by Harish Jenny K N

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



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

2018-03-02 15:28:10

by Linus Walleij

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

On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
<[email protected]> wrote:

> From: Andrew Gabbasov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Harish Jenny K N <[email protected]>

Makes sense to me, at least it makes the problem smaller not bigger.
Reviewed-by: Linus Walleij <[email protected]>

> 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

2018-03-06 12:50:06

by Harish Jenny K N

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



On Friday 02 March 2018 06:23 PM, Linus Walleij wrote:
> On Tue, Feb 27, 2018 at 12:33 PM, Harish Jenny K N
> <[email protected]> wrote:
>
>> From: Andrew Gabbasov <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Harish Jenny K N <[email protected]>
> Makes sense to me, at least it makes the problem smaller not bigger.
> Reviewed-by: Linus Walleij <[email protected]>


Any other comments/inputs on this ?


Thanks,
Harish Jenny K N


2018-03-08 20:37:19

by Alex Lemberg

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

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
> <[email protected]> wrote:
>
>> From: Andrew Gabbasov <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Harish Jenny K N <[email protected]>
> Makes sense to me, at least it makes the problem smaller not bigger.
> Reviewed-by: Linus Walleij <[email protected]>
>
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2018-03-10 12:00:29

by Linus Walleij

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

On Thu, Mar 8, 2018 at 9:36 PM, Alex Lemberg <[email protected]> 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

2018-03-12 04:45:35

by Harish Jenny K N

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



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

2018-04-04 12:47:59

by Ulf Hansson

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

On 27 February 2018 at 12:33, Harish Jenny K N
<[email protected]> wrote:
> From: Andrew Gabbasov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Harish Jenny K N <[email protected]>

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
>