| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <85b833aa-aa6d-4352-ac2c-08d988a55de3@linux.dev>
Date: Thu, 11 Dec 2025 15:54:11 +0800
From: Dongsheng Yang <dongsheng.yang@...ux.dev>
To: Li Chen <me@...ux.beauty>
Cc: dm-devel <dm-devel@...ts.linux.dev>,
linux-kernel <linux-kernel@...r.kernel.org>, Zheng Gu <cengku@...il.com>,
Mikulas Patocka <mpatocka@...hat.com>
Subject: Re: [PATCH] dm pcache: reject mappings larger than backing
在 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@...ux.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@...ux.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@...alhost ~# 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@...alhost ~ [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
>
>
Powered by blists - more mailing lists