[PATCH] dm pcache: reject mappings larger than backing

[PATCH] dm pcache: reject mappings larger than backing

[Li Chen]

Reject pcache targets whose logical size exceeds the backing device.
Prevent oversized tables from issuing IO past the end of the backing,
which may corrupt memory and cause kernel crash.

Signed-off-by: Li Chen <me@linux.beauty>
---
 drivers/md/dm-pcache/dm_pcache.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/md/dm-pcache/dm_pcache.c b/drivers/md/dm-pcache/dm_pcache.c
index e5f5936fa6f0..f72d1ba4b740 100644
--- a/drivers/md/dm-pcache/dm_pcache.c
+++ b/drivers/md/dm-pcache/dm_pcache.c
@@ -199,6 +199,8 @@ static int parse_cache_opts(struct dm_pcache *pcache, struct dm_arg_set *as,
 static int pcache_start(struct dm_pcache *pcache, char **error)
 {
 	int ret;
+	struct dm_target *ti = pcache->ti;
+	struct pcache_backing_dev *backing_dev;
 
 	ret = cache_dev_start(pcache);
 	if (ret) {
@@ -212,6 +214,19 @@ static int pcache_start(struct dm_pcache *pcache, char **error)
 		goto stop_cache;
 	}
 
+	/* Sanity-check: logical size must not exceed backing device size */
+	backing_dev = &pcache->backing_dev;
+	if (ti->len > backing_dev->dev_size) {
+		pcache_dev_err(
+			pcache,
+			"backing device too small: logical=%llu sectors, backing=%llu sectors",
+			(unsigned long long)ti->len,
+			(unsigned long long)backing_dev->dev_size);
+		*error = "Requested mapping exceeds backing device size";
+		ret = -EINVAL;
+		goto stop_backing;
+	}
+
 	ret = pcache_cache_start(pcache);
 	if (ret) {
 		*error = "Failed to start pcache";
-- 
2.51.0

Re: [PATCH] dm pcache: reject mappings larger than backing

[Dongsheng Yang]

在 12/8/2025 7:25 PM, Li Chen 写道:
> Reject pcache targets whose logical size exceeds the backing device.
> Prevent oversized tables from issuing IO past the end of the backing,
> which may corrupt memory and cause kernel crash.
>
> Signed-off-by: Li Chen <me@linux.beauty>
> ---
>   drivers/md/dm-pcache/dm_pcache.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/drivers/md/dm-pcache/dm_pcache.c b/drivers/md/dm-pcache/dm_pcache.c
> index e5f5936fa6f0..f72d1ba4b740 100644
> --- a/drivers/md/dm-pcache/dm_pcache.c
> +++ b/drivers/md/dm-pcache/dm_pcache.c
> @@ -199,6 +199,8 @@ static int parse_cache_opts(struct dm_pcache *pcache, struct dm_arg_set *as,
>   static int pcache_start(struct dm_pcache *pcache, char **error)
>   {
>   	int ret;
> +	struct dm_target *ti = pcache->ti;
> +	struct pcache_backing_dev *backing_dev;
>   
>   	ret = cache_dev_start(pcache);
>   	if (ret) {
> @@ -212,6 +214,19 @@ static int pcache_start(struct dm_pcache *pcache, char **error)
>   		goto stop_cache;
>   	}
>   
> +	/* Sanity-check: logical size must not exceed backing device size */
> +	backing_dev = &pcache->backing_dev;
> +	if (ti->len > backing_dev->dev_size) {
> +		pcache_dev_err(
> +			pcache,
> +			"backing device too small: logical=%llu sectors, backing=%llu sectors",
> +			(unsigned long long)ti->len,
> +			(unsigned long long)backing_dev->dev_size);
> +		*error = "Requested mapping exceeds backing device size";
> +		ret = -EINVAL;
> +		goto stop_backing;
> +	}
> +


Thanx for your patch, When developing dm-pcache, I considered whether to 
add a check there (for target size vs backing device size). But after 
looking at other existing targets, it seems none of them implement such 
a check, so I wasn't sure there is a requirement; hence I didn't add it 
at that time.

On the other hand, given that the “target size” parameter in the mapping 
table is a generic parameter, if we want such a check, it would be 
better to implement a generic mechanism in the Device-Mapper core 
construction path — provide a hook that allows every target to return a 
“maximum creatable target size”, and then in the core path validate the 
user-provided target size against that max size before creation.

That is just my personal idea; I hope Mikulas can provide more 
information about it.


Thanx

>   	ret = pcache_cache_start(pcache);
>   	if (ret) {
>   		*error = "Failed to start pcache";

Re: [PATCH] dm pcache: reject mappings larger than backing

[Li Chen]

Hi Dongsheng,

Thanks for your quick reply.
 ---- On Mon, 08 Dec 2025 19:44:40 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote --- 
 > 
 > 在 12/8/2025 7:25 PM, Li Chen 写道:
 > > Reject pcache targets whose logical size exceeds the backing device.
 > > Prevent oversized tables from issuing IO past the end of the backing,
 > > which may corrupt memory and cause kernel crash.
 > >
 > > Signed-off-by: Li Chen <me@linux.beauty>
 > > ---
 > >   drivers/md/dm-pcache/dm_pcache.c | 15 +++++++++++++++
 > >   1 file changed, 15 insertions(+)
 > >
 > > diff --git a/drivers/md/dm-pcache/dm_pcache.c b/drivers/md/dm-pcache/dm_pcache.c
 > > index e5f5936fa6f0..f72d1ba4b740 100644
 > > --- a/drivers/md/dm-pcache/dm_pcache.c
 > > +++ b/drivers/md/dm-pcache/dm_pcache.c
 > > @@ -199,6 +199,8 @@ static int parse_cache_opts(struct dm_pcache *pcache, struct dm_arg_set *as,
 > >   static int pcache_start(struct dm_pcache *pcache, char **error)
 > >   {
 > >       int ret;
 > > +    struct dm_target *ti = pcache->ti;
 > > +    struct pcache_backing_dev *backing_dev;
 > >   
 > >       ret = cache_dev_start(pcache);
 > >       if (ret) {
 > > @@ -212,6 +214,19 @@ static int pcache_start(struct dm_pcache *pcache, char **error)
 > >           goto stop_cache;
 > >       }
 > >   
 > > +    /* Sanity-check: logical size must not exceed backing device size */
 > > +    backing_dev = &pcache->backing_dev;
 > > +    if (ti->len > backing_dev->dev_size) {
 > > +        pcache_dev_err(
 > > +            pcache,
 > > +            "backing device too small: logical=%llu sectors, backing=%llu sectors",
 > > +            (unsigned long long)ti->len,
 > > +            (unsigned long long)backing_dev->dev_size);
 > > +        *error = "Requested mapping exceeds backing device size";
 > > +        ret = -EINVAL;
 > > +        goto stop_backing;
 > > +    }
 > > +
 > 
 > 
 > Thanx for your patch, When developing dm-pcache, I considered whether to 
 > add a check there (for target size vs backing device size). But after 
 > looking at other existing targets, it seems none of them implement such 
 > a check, so I wasn't sure there is a requirement; hence I didn't add it 
 > at that time.

 I did a quick search and found some examples:
https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-cache-target.c#L2117
https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-verity-target.c#L1416
https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-integrity.c#L5166

 > On the other hand, given that the “target size” parameter in the mapping 
 > table is a generic parameter, if we want such a check, it would be 
 > better to implement a generic mechanism in the Device-Mapper core 
 > construction path — provide a hook that allows every target to return a 
 > “maximum creatable target size”, and then in the core path validate the 
 > user-provided target size against that max size before creation.
 > 
 > That is just my personal idea; I hope Mikulas can provide more 
 > information about it.

I just found a common helper called device_area_is_invalid check, already used within target_type->iterate_devices, validates target size. 
Therefore, targets implementing iterate_devices can perform size validation. I confirmed this with a quick test:

modified   drivers/md/dm-pcache/dm_pcache.c
@@ -446,6 +446,15 @@ static int dm_pcache_message(struct dm_target *ti, unsigned int argc,
 	return -EINVAL;
 }
 
+static int pcache_iterate_devices(struct dm_target *ti,
+				  iterate_devices_callout_fn fn, void *data)
+{
+	struct dm_pcache *pcache = ti->private;
+	struct pcache_backing_dev *backing_dev = &pcache->backing_dev;
+
+	return fn(ti, backing_dev->dm_dev, 0, ti->len, data);
+}
+
 static struct target_type dm_pcache_target = {
 	.name		= "pcache",
 	.version	= {0, 1, 0},
@@ -456,6 +465,7 @@ static struct target_type dm_pcache_target = {
 	.map		= dm_pcache_map_bio,
 	.status		= dm_pcache_status,
 	.message	= dm_pcache_message,
+	.iterate_devices = pcache_iterate_devices,
 };
 
 static int __init dm_pcache_init(void)

And it is rejected as expected too:

```
root@localhost ~# dmsetup create pcache_vdb --table \
                                                                                              "0 31457284 pcache /dev/pmem0 /dev/vdb 4 cache_mode writeback data_crc false"
device-mapper: reload ioctl on pcache_vdb (253:0) failed: Invalid argument
Complete cookie 0xd4d9a60 success.
Complete cookie 0xd4d9a60 success.
Command failed.
root@localhost ~ [1]# dmesg | tail
[    1.937147] EXT4-fs (vda2): re-mounted dc39223f-5278-4b19-80bc-de1edf4a5a96 r/w.
[    1.941399] systemd-journald[166]: Received client request to flush runtime journal.
[    2.051496] virtio_net virtio0 enp0s3: renamed from eth0
[    2.253929] FAT-fs (vda1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
[    2.395796] tsc: Refined TSC clocksource calibration: 3792.905 MHz
[    2.395834] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x6d585308801, max_idle_ns: 881590710907 ns
[    2.396195] clocksource: Switched to clocksource tsc
[  175.269911] device-mapper: table: 253:0: vdb too small for target: start=0, len=31457284, dev_size=31457280
[  175.271582] device-mapper: core: Cannot calculate initial queue limits
[  175.272688] device-mapper: ioctl: unable to set up device queue for new table.
```

Which do you prefer: pcache_iterate_devices or checking in parse_cache_opts? Thanks.

Li​

Re: [PATCH] dm pcache: reject mappings larger than backing

[Dongsheng Yang]

在 12/10/2025 10:26 AM, Li Chen 写道:
> Hi Dongsheng,
>
> Thanks for your quick reply.
>   ---- On Mon, 08 Dec 2025 19:44:40 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote ---
>   >
>   > 在 12/8/2025 7:25 PM, Li Chen 写道:
>   > > Reject pcache targets whose logical size exceeds the backing device.
>   > > Prevent oversized tables from issuing IO past the end of the backing,
>   > > which may corrupt memory and cause kernel crash.
>   > >
>   > > Signed-off-by: Li Chen <me@linux.beauty>
>   > > ---
>   > >   drivers/md/dm-pcache/dm_pcache.c | 15 +++++++++++++++
>   > >   1 file changed, 15 insertions(+)
>   > >
>   > > diff --git a/drivers/md/dm-pcache/dm_pcache.c b/drivers/md/dm-pcache/dm_pcache.c
>   > > index e5f5936fa6f0..f72d1ba4b740 100644
>   > > --- a/drivers/md/dm-pcache/dm_pcache.c
>   > > +++ b/drivers/md/dm-pcache/dm_pcache.c
>   > > @@ -199,6 +199,8 @@ static int parse_cache_opts(struct dm_pcache *pcache, struct dm_arg_set *as,
>   > >   static int pcache_start(struct dm_pcache *pcache, char **error)
>   > >   {
>   > >       int ret;
>   > > +    struct dm_target *ti = pcache->ti;
>   > > +    struct pcache_backing_dev *backing_dev;
>   > >
>   > >       ret = cache_dev_start(pcache);
>   > >       if (ret) {
>   > > @@ -212,6 +214,19 @@ static int pcache_start(struct dm_pcache *pcache, char **error)
>   > >           goto stop_cache;
>   > >       }
>   > >
>   > > +    /* Sanity-check: logical size must not exceed backing device size */
>   > > +    backing_dev = &pcache->backing_dev;
>   > > +    if (ti->len > backing_dev->dev_size) {
>   > > +        pcache_dev_err(
>   > > +            pcache,
>   > > +            "backing device too small: logical=%llu sectors, backing=%llu sectors",
>   > > +            (unsigned long long)ti->len,
>   > > +            (unsigned long long)backing_dev->dev_size);
>   > > +        *error = "Requested mapping exceeds backing device size";
>   > > +        ret = -EINVAL;
>   > > +        goto stop_backing;
>   > > +    }
>   > > +
>   >
>   >
>   > Thanx for your patch, When developing dm-pcache, I considered whether to
>   > add a check there (for target size vs backing device size). But after
>   > looking at other existing targets, it seems none of them implement such
>   > a check, so I wasn't sure there is a requirement; hence I didn't add it
>   > at that time.
>
>   I did a quick search and found some examples:
> https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-cache-target.c#L2117
> https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-verity-target.c#L1416
> https://github.com/torvalds/linux/blob/c9b47175e9131118e6f221cc8fb81397d62e7c91/drivers/md/dm-integrity.c#L5166
>
>   > On the other hand, given that the “target size” parameter in the mapping
>   > table is a generic parameter, if we want such a check, it would be
>   > better to implement a generic mechanism in the Device-Mapper core
>   > construction path — provide a hook that allows every target to return a
>   > “maximum creatable target size”, and then in the core path validate the
>   > user-provided target size against that max size before creation.
>   >
>   > That is just my personal idea; I hope Mikulas can provide more
>   > information about it.
>
> I just found a common helper called device_area_is_invalid check, already used within target_type->iterate_devices, validates target size.
> Therefore, targets implementing iterate_devices can perform size validation. I confirmed this with a quick test:
>
> modified   drivers/md/dm-pcache/dm_pcache.c
> @@ -446,6 +446,15 @@ static int dm_pcache_message(struct dm_target *ti, unsigned int argc,
>   	return -EINVAL;
>   }
>   
> +static int pcache_iterate_devices(struct dm_target *ti,
> +				  iterate_devices_callout_fn fn, void *data)
> +{
> +	struct dm_pcache *pcache = ti->private;
> +	struct pcache_backing_dev *backing_dev = &pcache->backing_dev;
> +
> +	return fn(ti, backing_dev->dm_dev, 0, ti->len, data);
> +}
> +
>   static struct target_type dm_pcache_target = {
>   	.name		= "pcache",
>   	.version	= {0, 1, 0},
> @@ -456,6 +465,7 @@ static struct target_type dm_pcache_target = {
>   	.map		= dm_pcache_map_bio,
>   	.status		= dm_pcache_status,
>   	.message	= dm_pcache_message,
> +	.iterate_devices = pcache_iterate_devices,
>   };
>   
>   static int __init dm_pcache_init(void)
>
> And it is rejected as expected too:
>
> ```
> root@localhost ~# dmsetup create pcache_vdb --table \
>                                                                                                "0 31457284 pcache /dev/pmem0 /dev/vdb 4 cache_mode writeback data_crc false"
> device-mapper: reload ioctl on pcache_vdb (253:0) failed: Invalid argument
> Complete cookie 0xd4d9a60 success.
> Complete cookie 0xd4d9a60 success.
> Command failed.
> root@localhost ~ [1]# dmesg | tail
> [    1.937147] EXT4-fs (vda2): re-mounted dc39223f-5278-4b19-80bc-de1edf4a5a96 r/w.
> [    1.941399] systemd-journald[166]: Received client request to flush runtime journal.
> [    2.051496] virtio_net virtio0 enp0s3: renamed from eth0
> [    2.253929] FAT-fs (vda1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
> [    2.395796] tsc: Refined TSC clocksource calibration: 3792.905 MHz
> [    2.395834] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x6d585308801, max_idle_ns: 881590710907 ns
> [    2.396195] clocksource: Switched to clocksource tsc
> [  175.269911] device-mapper: table: 253:0: vdb too small for target: start=0, len=31457284, dev_size=31457280
> [  175.271582] device-mapper: core: Cannot calculate initial queue limits
> [  175.272688] device-mapper: ioctl: unable to set up device queue for new table.
> ```
>
> Which do you prefer: pcache_iterate_devices or checking in parse_cache_opts? Thanks.


iterate_devices() sounds not the proper place to do size checking. If there is no generic way to do size checking, I prefer to do it in parse_backing_dev() rather than pcache_iterate_devices.

Thanx


>
> Li​
>
>