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

Li Chen
2025-12-10T02:26:28Z

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​