[PATCH 0/3] dm-pcache: built-in support and metadata hardening

[PATCH 0/3] dm-pcache: built-in support and metadata hardening

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

This three-patch series tidies dm-pcache’s build glue and tightens the metadata scan.

Patch 1 allow dm-pcache to be linked into vmlinux and avoids clashing with the sunrpc
cache_flush() by using obj-$(CONFIG_DM_PCACHE) and renaming the helper across the tree.

Patch 2 drops a redundant recomputation of the metadata slot pointer while walking headers.

Patch 3 zero-allocates a temporary buffer so callers never see stale metadata,
relies on __free(kvfree) for cleanup, and only copies back once a valid record is found.

Thanks for your review.

Li Chen (3):
  dm-pcache: allow built-in build and rename flush helper
  dm-pcache: reuse meta_addr in pcache_meta_find_latest
  dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

 drivers/md/dm-pcache/Makefile          |  2 +-
 drivers/md/dm-pcache/cache.c           |  2 +-
 drivers/md/dm-pcache/cache.h           |  2 +-
 drivers/md/dm-pcache/cache_req.c       |  6 +++---
 drivers/md/dm-pcache/pcache_internal.h | 15 ++++++++++-----
 5 files changed, 16 insertions(+), 11 deletions(-)

-- 
2.51.0

[PATCH 1/3] dm-pcache: allow built-in build and rename flush helper

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

CONFIG_BCACHE is tristate, so dm-pcache can also be built-in.
Switch the Makefile to use obj-$(CONFIG_DM_PCACHE) so the target can be
linked into vmlinux instead of always being a loadable module.

Also rename cache_flush() to pcache_cache_flush() to avoid a global
symbol clash with sunrpc/cache.c's cache_flush().

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 drivers/md/dm-pcache/Makefile    | 2 +-
 drivers/md/dm-pcache/cache.c     | 2 +-
 drivers/md/dm-pcache/cache.h     | 2 +-
 drivers/md/dm-pcache/cache_req.c | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/md/dm-pcache/Makefile b/drivers/md/dm-pcache/Makefile
index 86776e4acad24..cedfd38854f63 100644
--- a/drivers/md/dm-pcache/Makefile
+++ b/drivers/md/dm-pcache/Makefile
@@ -1,3 +1,3 @@
 dm-pcache-y := dm_pcache.o cache_dev.o segment.o backing_dev.o cache.o cache_gc.o cache_writeback.o cache_segment.o cache_key.o cache_req.o
 
-obj-m += dm-pcache.o
+obj-$(CONFIG_DM_PCACHE) += dm-pcache.o
diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index d8e92367d9470..d516d49042272 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -411,7 +411,7 @@ void pcache_cache_stop(struct dm_pcache *pcache)
 {
 	struct pcache_cache *cache = &pcache->cache;
 
-	cache_flush(cache);
+	pcache_cache_flush(cache);
 
 	cancel_delayed_work_sync(&cache->gc_work);
 	flush_work(&cache->clean_work);
diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
index 1136d86958c8c..27613b56be54c 100644
--- a/drivers/md/dm-pcache/cache.h
+++ b/drivers/md/dm-pcache/cache.h
@@ -339,7 +339,7 @@ void cache_seg_put(struct pcache_cache_segment *cache_seg);
 void cache_seg_set_next_seg(struct pcache_cache_segment *cache_seg, u32 seg_id);
 
 /* cache request*/
-int cache_flush(struct pcache_cache *cache);
+int pcache_cache_flush(struct pcache_cache *cache);
 void miss_read_end_work_fn(struct work_struct *work);
 int pcache_cache_handle_req(struct pcache_cache *cache, struct pcache_request *pcache_req);
 
diff --git a/drivers/md/dm-pcache/cache_req.c b/drivers/md/dm-pcache/cache_req.c
index 27f94c1fa968c..7854a30e07b7f 100644
--- a/drivers/md/dm-pcache/cache_req.c
+++ b/drivers/md/dm-pcache/cache_req.c
@@ -790,7 +790,7 @@ static int cache_write(struct pcache_cache *cache, struct pcache_request *pcache
 }
 
 /**
- * cache_flush - Flush all ksets to persist any pending cache data
+ * pcache_cache_flush - Flush all ksets to persist any pending cache data
  * @cache: Pointer to the cache structure
  *
  * This function iterates through all ksets associated with the provided `cache`
@@ -802,7 +802,7 @@ static int cache_write(struct pcache_cache *cache, struct pcache_request *pcache
  * the respective error code, preventing the flush operation from proceeding to
  * subsequent ksets.
  */
-int cache_flush(struct pcache_cache *cache)
+int pcache_cache_flush(struct pcache_cache *cache)
 {
 	struct pcache_cache_kset *kset;
 	int ret;
@@ -827,7 +827,7 @@ int pcache_cache_handle_req(struct pcache_cache *cache, struct pcache_request *p
 	struct bio *bio = pcache_req->bio;
 
 	if (unlikely(bio->bi_opf & REQ_PREFLUSH))
-		return cache_flush(cache);
+		return pcache_cache_flush(cache);
 
 	if (bio_data_dir(bio) == READ)
 		return cache_read(cache, pcache_req);
-- 
2.51.0

[PATCH 2/3] dm-pcache: reuse meta_addr in pcache_meta_find_latest

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

pcache_meta_find_latest() already computes the metadata address as
meta_addr. Reuse that instead of recomputing.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 drivers/md/dm-pcache/pcache_internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
index d427e534727ce..b7a3319d2bd3e 100644
--- a/drivers/md/dm-pcache/pcache_internal.h
+++ b/drivers/md/dm-pcache/pcache_internal.h
@@ -99,7 +99,7 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
 		/* Update latest if a more recent sequence is found */
 		if (!latest || pcache_meta_seq_after(meta->seq, seq_latest)) {
 			seq_latest = meta->seq;
-			latest = (void *)header + (i * meta_max_size);
+			latest = meta_addr;
 		}
 	}
 
-- 
2.51.0

[PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

Before this change pcache_meta_find_latest() was copying each
slot directly into meta_ret while scanning. If no valid slot
was found and the function returned NULL, meta_ret still held
whatever was last copied (possibly CRC-bad). Later users
(e.g. cache_segs_init) could mistakenly trust that data.

Allocate a temporary buffer instead and only populate meta_ret after a
valid/latest header is found. If no valid header exists we return NULL
without touching meta_ret.

Also add __free(kvfree) so the temporary buffer is always freed, and
include the needed headers.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 drivers/md/dm-pcache/pcache_internal.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
index b7a3319d2bd3e..ac28f9dd2986f 100644
--- a/drivers/md/dm-pcache/pcache_internal.h
+++ b/drivers/md/dm-pcache/pcache_internal.h
@@ -4,6 +4,8 @@
 
 #include <linux/delay.h>
 #include <linux/crc32c.h>
+#include <linux/slab.h>
+#include <linux/cleanup.h>
 
 #define pcache_err(fmt, ...)							\
 	pr_err("dm-pcache: %s:%u " fmt, __func__, __LINE__, ##__VA_ARGS__)
@@ -79,14 +81,17 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
 					u32 meta_size, u32 meta_max_size,
 					void *meta_ret)
 {
-	struct pcache_meta_header *meta, *latest = NULL;
+	struct pcache_meta_header *latest = NULL;
+	struct pcache_meta_header *meta __free(kvfree);
 	u32 i, seq_latest = 0;
-	void *meta_addr;
 
-	meta = meta_ret;
+	meta = kvzalloc(meta_size, GFP_KERNEL);
+	if (!meta)
+		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < PCACHE_META_INDEX_MAX; i++) {
-		meta_addr = (void *)header + (i * meta_max_size);
+		void *meta_addr = (void *)header + (i * meta_max_size);
+
 		if (copy_mc_to_kernel(meta, meta_addr, meta_size)) {
 			pcache_err("hardware memory error when copy meta");
 			return ERR_PTR(-EIO);
-- 
2.51.0

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

Hi Zheng,

 ---- On Fri, 31 Oct 2025 10:01:23 +0800  Zheng Gu <cengku@gmail.com> wrote --- 
 >> On Thu, Oct 30, 2025 at 8:36 PM Li Chen <me@linux.beauty> wrote:From: Li Chen <chenl311@chinatelecom.cn>
 >> 
 >> Before this change pcache_meta_find_latest() was copying each
 >> slot directly into meta_ret while scanning. If no valid slot
 >> was found and the function returned NULL, meta_ret still held
 >> whatever was last copied (possibly CRC-bad). Later users
 >> (e.g. cache_segs_init) could mistakenly trust that data.
 > 
 > This functions is * __must_check*, users must check the return value first before touching the meta_ret, so it should not be a problem here.

Right now, the callers only check the return value with IS_ERR(). If the
function returns NULL instead of an error pointer, a caller like
cache_info_init() will assume that no valid cache_info was found because all cache_info are
corrupted. Instead, it will try to init a new one, and then return 0 (success), 
https://github.com/torvalds/linux/blob/master/drivers/md/dm-pcache/cache.c#L61

Later, cache_tail_init() will access cache->cache_info.flags. But in this
path all cache_info may have already been corrupted, and the CRCs are mismatched
(https://github.com/torvalds/linux/blob/ba36dd5ee6fd4643ebbf6ee6eefcecf0b07e35c7/drivers/md/dm-pcache/pcache_internal.h#L97),
so flags may contain garbage.

This commit fixes this issue by allocating a temp buffer with kvmalloc, so meta_ret would never
contain corrupted values.

Regards,

Li​

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Dongsheng Yang]

在 11/1/2025 9:10 PM, Li Chen 写道:
> Hi Zheng,
>
>   ---- On Fri, 31 Oct 2025 10:01:23 +0800  Zheng Gu <cengku@gmail.com> wrote ---
>   >> On Thu, Oct 30, 2025 at 8:36 PM Li Chen <me@linux.beauty> wrote:From: Li Chen <chenl311@chinatelecom.cn>
>   >>
>   >> Before this change pcache_meta_find_latest() was copying each
>   >> slot directly into meta_ret while scanning. If no valid slot
>   >> was found and the function returned NULL, meta_ret still held
>   >> whatever was last copied (possibly CRC-bad). Later users
>   >> (e.g. cache_segs_init) could mistakenly trust that data.
>   >
>   > This functions is * __must_check*, users must check the return value first before touching the meta_ret, so it should not be a problem here.
>
> Right now, the callers only check the return value with IS_ERR(). If the
> function returns NULL instead of an error pointer, a caller like
> cache_info_init() will assume that no valid cache_info was found because all cache_info are
> corrupted. Instead, it will try to init a new one, and then return 0 (success),
> https://github.com/torvalds/linux/blob/master/drivers/md/dm-pcache/cache.c#L61
>
> Later, cache_tail_init() will access cache->cache_info.flags. But in this
> path all cache_info may have already been corrupted, and the CRCs are mismatched
> (https://github.com/torvalds/linux/blob/ba36dd5ee6fd4643ebbf6ee6eefcecf0b07e35c7/drivers/md/dm-pcache/pcache_internal.h#L97),
> so flags may contain garbage.
>
> This commit fixes this issue by allocating a temp buffer with kvmalloc, so meta_ret would never
> contain corrupted values.

Hi

     Thanx for your fix. So the better change should be reseting 
cache_info in cache_info_init_default() firstly by memset() with 0.

Allocating a temp buffer in pcache_meta_find_latest() is really not a 
good idea.

Thanx

>
> Regards,
>
> Li​
>
>

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

Hi Dongsheng,


 ---- On Tue, 04 Nov 2025 14:46:33 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote --- 
 > 
 > 在 11/1/2025 9:10 PM, Li Chen 写道:
 > > Hi Zheng,
 > >
 > >   ---- On Fri, 31 Oct 2025 10:01:23 +0800  Zheng Gu <cengku@gmail.com> wrote ---
 > >   >> On Thu, Oct 30, 2025 at 8:36 PM Li Chen <me@linux.beauty> wrote:From: Li Chen <chenl311@chinatelecom.cn>
 > >   >>
 > >   >> Before this change pcache_meta_find_latest() was copying each
 > >   >> slot directly into meta_ret while scanning. If no valid slot
 > >   >> was found and the function returned NULL, meta_ret still held
 > >   >> whatever was last copied (possibly CRC-bad). Later users
 > >   >> (e.g. cache_segs_init) could mistakenly trust that data.
 > >   >
 > >   > This functions is * __must_check*, users must check the return value first before touching the meta_ret, so it should not be a problem here.
 > >
 > > Right now, the callers only check the return value with IS_ERR(). If the
 > > function returns NULL instead of an error pointer, a caller like
 > > cache_info_init() will assume that no valid cache_info was found because all cache_info are
 > > corrupted. Instead, it will try to init a new one, and then return 0 (success),
 > > https://github.com/torvalds/linux/blob/master/drivers/md/dm-pcache/cache.c#L61
 > >
 > > Later, cache_tail_init() will access cache->cache_info.flags. But in this
 > > path all cache_info may have already been corrupted, and the CRCs are mismatched
 > > (https://github.com/torvalds/linux/blob/ba36dd5ee6fd4643ebbf6ee6eefcecf0b07e35c7/drivers/md/dm-pcache/pcache_internal.h#L97),
 > > so flags may contain garbage.
 > >
 > > This commit fixes this issue by allocating a temp buffer with kvmalloc, so meta_ret would never
 > > contain corrupted values.
 > 
 > Hi
 > 
 >      Thanx for your fix. So the better change should be reseting 
 > cache_info in cache_info_init_default() firstly by memset() with 0.
 > 
 > Allocating a temp buffer in pcache_meta_find_latest() is really not a 
 > good idea.

I considered using memset before sending the patch, but a temporary buffer seems more elegant. 
Since the variable is relatively large, I avoided stack allocation. If you prefer memset, should it be implemented 
within pcache_meta_find_latest or all its callers?

Regards,

Li​

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Dongsheng Yang]

在 11/4/2025 9:36 PM, Li Chen 写道:
> Hi Dongsheng,
>
>
>   ---- On Tue, 04 Nov 2025 14:46:33 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote ---
>   >
>   > 在 11/1/2025 9:10 PM, Li Chen 写道:
>   > > Hi Zheng,
>   > >
>   > >   ---- On Fri, 31 Oct 2025 10:01:23 +0800  Zheng Gu <cengku@gmail.com> wrote ---
>   > >   >> On Thu, Oct 30, 2025 at 8:36 PM Li Chen <me@linux.beauty> wrote:From: Li Chen <chenl311@chinatelecom.cn>
>   > >   >>
>   > >   >> Before this change pcache_meta_find_latest() was copying each
>   > >   >> slot directly into meta_ret while scanning. If no valid slot
>   > >   >> was found and the function returned NULL, meta_ret still held
>   > >   >> whatever was last copied (possibly CRC-bad). Later users
>   > >   >> (e.g. cache_segs_init) could mistakenly trust that data.
>   > >   >
>   > >   > This functions is * __must_check*, users must check the return value first before touching the meta_ret, so it should not be a problem here.
>   > >
>   > > Right now, the callers only check the return value with IS_ERR(). If the
>   > > function returns NULL instead of an error pointer, a caller like
>   > > cache_info_init() will assume that no valid cache_info was found because all cache_info are
>   > > corrupted. Instead, it will try to init a new one, and then return 0 (success),
>   > > https://github.com/torvalds/linux/blob/master/drivers/md/dm-pcache/cache.c#L61
>   > >
>   > > Later, cache_tail_init() will access cache->cache_info.flags. But in this
>   > > path all cache_info may have already been corrupted, and the CRCs are mismatched
>   > > (https://github.com/torvalds/linux/blob/ba36dd5ee6fd4643ebbf6ee6eefcecf0b07e35c7/drivers/md/dm-pcache/pcache_internal.h#L97),
>   > > so flags may contain garbage.
>   > >
>   > > This commit fixes this issue by allocating a temp buffer with kvmalloc, so meta_ret would never
>   > > contain corrupted values.
>   >
>   > Hi
>   >
>   >      Thanx for your fix. So the better change should be reseting
>   > cache_info in cache_info_init_default() firstly by memset() with 0.
>   >
>   > Allocating a temp buffer in pcache_meta_find_latest() is really not a
>   > good idea.
>
> I considered using memset before sending the patch, but a temporary buffer seems more elegant.
> Since the variable is relatively large, I avoided stack allocation. If you prefer memset, should it be implemented
> within pcache_meta_find_latest or all its callers?


callers should do this thing, it's about default value initialization, 
the callers understand what to do, but pcache_meta_find_latest() does not.

So the usage looks like below:

         meta = pcache_meta_find_latest();

If meta is error, return error.

If meta is not NULL, meta is valid, just use it.

If meta is NULL, that means there is no valid meta onmedia, just init it 
with default value (including cache_info.flags you mentioned, the 
default of this flags should be 0).


BTW, when you memset cache_info with 0 in cache_info_init_default();, 
you can remove this line: cache_info->header.seq = 0;

Thanx

>
> Regards,
>
> Li​
>

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Jonathan Cameron]

On Thu, 30 Oct 2025 20:33:21 +0800
Li Chen <me@linux.beauty> wrote:

> From: Li Chen <chenl311@chinatelecom.cn>
> 
> Before this change pcache_meta_find_latest() was copying each
> slot directly into meta_ret while scanning. If no valid slot
> was found and the function returned NULL, meta_ret still held
> whatever was last copied (possibly CRC-bad). Later users
> (e.g. cache_segs_init) could mistakenly trust that data.
> 
> Allocate a temporary buffer instead and only populate meta_ret after a
> valid/latest header is found. If no valid header exists we return NULL
> without touching meta_ret.
> 
> Also add __free(kvfree) so the temporary buffer is always freed, and
> include the needed headers.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> ---
>  drivers/md/dm-pcache/pcache_internal.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
> index b7a3319d2bd3e..ac28f9dd2986f 100644
> --- a/drivers/md/dm-pcache/pcache_internal.h
> +++ b/drivers/md/dm-pcache/pcache_internal.h
> @@ -4,6 +4,8 @@
>  
>  #include <linux/delay.h>
>  #include <linux/crc32c.h>
> +#include <linux/slab.h>
> +#include <linux/cleanup.h>
>  
>  #define pcache_err(fmt, ...)							\
>  	pr_err("dm-pcache: %s:%u " fmt, __func__, __LINE__, ##__VA_ARGS__)
> @@ -79,14 +81,17 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
>  					u32 meta_size, u32 meta_max_size,
>  					void *meta_ret)
>  {
> -	struct pcache_meta_header *meta, *latest = NULL;
> +	struct pcache_meta_header *latest = NULL;
> +	struct pcache_meta_header *meta __free(kvfree);
>  	u32 i, seq_latest = 0;
> -	void *meta_addr;
>  
> -	meta = meta_ret;
> +	meta = kvzalloc(meta_size, GFP_KERNEL);
See the guidance notes in cleanup.h  THis hsould be

	struct pcache_meta_header *meta __free(kvfree) =
		kvzalloc(meta_size, GFP_KERNEL);

That is the constructor and destructor must be together. Inline variable
declarations are fine for this one type of use.

> +	if (!meta)
> +		return ERR_PTR(-ENOMEM);
>  
>  	for (i = 0; i < PCACHE_META_INDEX_MAX; i++) {
> -		meta_addr = (void *)header + (i * meta_max_size);
> +		void *meta_addr = (void *)header + (i * meta_max_size);
> +
>  		if (copy_mc_to_kernel(meta, meta_addr, meta_size)) {
>  			pcache_err("hardware memory error when copy meta");
>  			return ERR_PTR(-EIO);

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

Hi Jonathan,

 ---- On Mon, 03 Nov 2025 19:38:03 +0800  Jonathan Cameron <jonathan.cameron@huawei.com> wrote --- 
 > On Thu, 30 Oct 2025 20:33:21 +0800
 > Li Chen <me@linux.beauty> wrote:
 > 
 > > From: Li Chen <chenl311@chinatelecom.cn>
 > > 
 > > Before this change pcache_meta_find_latest() was copying each
 > > slot directly into meta_ret while scanning. If no valid slot
 > > was found and the function returned NULL, meta_ret still held
 > > whatever was last copied (possibly CRC-bad). Later users
 > > (e.g. cache_segs_init) could mistakenly trust that data.
 > > 
 > > Allocate a temporary buffer instead and only populate meta_ret after a
 > > valid/latest header is found. If no valid header exists we return NULL
 > > without touching meta_ret.
 > > 
 > > Also add __free(kvfree) so the temporary buffer is always freed, and
 > > include the needed headers.
 > > 
 > > Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
 > > ---
 > >  drivers/md/dm-pcache/pcache_internal.h | 13 +++++++++----
 > >  1 file changed, 9 insertions(+), 4 deletions(-)
 > > 
 > > diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
 > > index b7a3319d2bd3e..ac28f9dd2986f 100644
 > > --- a/drivers/md/dm-pcache/pcache_internal.h
 > > +++ b/drivers/md/dm-pcache/pcache_internal.h
 > > @@ -4,6 +4,8 @@
 > >  
 > >  #include <linux/delay.h>
 > >  #include <linux/crc32c.h>
 > > +#include <linux/slab.h>
 > > +#include <linux/cleanup.h>
 > >  
 > >  #define pcache_err(fmt, ...)                            \
 > >      pr_err("dm-pcache: %s:%u " fmt, __func__, __LINE__, ##__VA_ARGS__)
 > > @@ -79,14 +81,17 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
 > >                      u32 meta_size, u32 meta_max_size,
 > >                      void *meta_ret)
 > >  {
 > > -    struct pcache_meta_header *meta, *latest = NULL;
 > > +    struct pcache_meta_header *latest = NULL;
 > > +    struct pcache_meta_header *meta __free(kvfree);
 > >      u32 i, seq_latest = 0;
 > > -    void *meta_addr;
 > >  
 > > -    meta = meta_ret;
 > > +    meta = kvzalloc(meta_size, GFP_KERNEL);
 > See the guidance notes in cleanup.h  THis hsould be
 > 
 >     struct pcache_meta_header *meta __free(kvfree) =
 >         kvzalloc(meta_size, GFP_KERNEL);
 > 
 > That is the constructor and destructor must be together. Inline variable
 > declarations are fine for this one type of use.

I appreciate your suggestion very much.

Regards,

Li​

[PATCH 0/3] dm-pcache: built-in support and metadata hardening

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

This three-patch series tidies dm-pcache’s build glue and tightens the metadata scan.

Patch 1 allow dm-pcache to be linked into vmlinux and avoids clashing with the sunrpc
cache_flush() by using obj-$(CONFIG_DM_PCACHE) and renaming the helper across the tree.

Patch 2 drops a redundant recomputation of the metadata slot pointer while walking headers.

Patch 3 zero-allocates a temporary buffer so callers never see stale metadata,
relies on __free(kvfree) for cleanup, and only copies back once a valid record is found.

Thanks for your review.

Li Chen (3):
  dm-pcache: allow built-in build and rename flush helper
  dm-pcache: reuse meta_addr in pcache_meta_find_latest
  dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

 drivers/md/dm-pcache/Makefile          |  2 +-
 drivers/md/dm-pcache/cache.c           |  2 +-
 drivers/md/dm-pcache/cache.h           |  2 +-
 drivers/md/dm-pcache/cache_req.c       |  6 +++---
 drivers/md/dm-pcache/pcache_internal.h | 15 ++++++++++-----
 5 files changed, 16 insertions(+), 11 deletions(-)

-- 
2.51.0

[PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

Before this change pcache_meta_find_latest() was copying each
slot directly into meta_ret while scanning. If no valid slot
was found and the function returned NULL, meta_ret still held
whatever was last copied (possibly CRC-bad). Later users
(e.g. cache_segs_init) could mistakenly trust that data.

Allocate a temporary buffer instead and only populate meta_ret after a
valid/latest header is found. If no valid header exists we return NULL
without touching meta_ret.

Also add __free(kvfree) so the temporary buffer is always freed, and
include the needed headers.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 drivers/md/dm-pcache/pcache_internal.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
index b7a3319d2bd3e..ac28f9dd2986f 100644
--- a/drivers/md/dm-pcache/pcache_internal.h
+++ b/drivers/md/dm-pcache/pcache_internal.h
@@ -4,6 +4,8 @@
 
 #include <linux/delay.h>
 #include <linux/crc32c.h>
+#include <linux/slab.h>
+#include <linux/cleanup.h>
 
 #define pcache_err(fmt, ...)							\
 	pr_err("dm-pcache: %s:%u " fmt, __func__, __LINE__, ##__VA_ARGS__)
@@ -79,14 +81,17 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
 					u32 meta_size, u32 meta_max_size,
 					void *meta_ret)
 {
-	struct pcache_meta_header *meta, *latest = NULL;
+	struct pcache_meta_header *latest = NULL;
+	struct pcache_meta_header *meta __free(kvfree);
 	u32 i, seq_latest = 0;
-	void *meta_addr;
 
-	meta = meta_ret;
+	meta = kvzalloc(meta_size, GFP_KERNEL);
+	if (!meta)
+		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < PCACHE_META_INDEX_MAX; i++) {
-		meta_addr = (void *)header + (i * meta_max_size);
+		void *meta_addr = (void *)header + (i * meta_max_size);
+
 		if (copy_mc_to_kernel(meta, meta_addr, meta_size)) {
 			pcache_err("hardware memory error when copy meta");
 			return ERR_PTR(-EIO);
-- 
2.51.0

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Dongsheng Yang]

Hi Li,

     It seems you sent the same patch again, shoud it be V2 instead?

Thanx

在 11/5/2025 4:46 PM, Li Chen 写道:
> From: Li Chen <chenl311@chinatelecom.cn>
>
> Before this change pcache_meta_find_latest() was copying each
> slot directly into meta_ret while scanning. If no valid slot
> was found and the function returned NULL, meta_ret still held
> whatever was last copied (possibly CRC-bad). Later users
> (e.g. cache_segs_init) could mistakenly trust that data.
>
> Allocate a temporary buffer instead and only populate meta_ret after a
> valid/latest header is found. If no valid header exists we return NULL
> without touching meta_ret.
>
> Also add __free(kvfree) so the temporary buffer is always freed, and
> include the needed headers.
>
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> ---
>   drivers/md/dm-pcache/pcache_internal.h | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-pcache/pcache_internal.h b/drivers/md/dm-pcache/pcache_internal.h
> index b7a3319d2bd3e..ac28f9dd2986f 100644
> --- a/drivers/md/dm-pcache/pcache_internal.h
> +++ b/drivers/md/dm-pcache/pcache_internal.h
> @@ -4,6 +4,8 @@
>   
>   #include <linux/delay.h>
>   #include <linux/crc32c.h>
> +#include <linux/slab.h>
> +#include <linux/cleanup.h>
>   
>   #define pcache_err(fmt, ...)							\
>   	pr_err("dm-pcache: %s:%u " fmt, __func__, __LINE__, ##__VA_ARGS__)
> @@ -79,14 +81,17 @@ static inline void __must_check *pcache_meta_find_latest(struct pcache_meta_head
>   					u32 meta_size, u32 meta_max_size,
>   					void *meta_ret)
>   {
> -	struct pcache_meta_header *meta, *latest = NULL;
> +	struct pcache_meta_header *latest = NULL;
> +	struct pcache_meta_header *meta __free(kvfree);
>   	u32 i, seq_latest = 0;
> -	void *meta_addr;
>   
> -	meta = meta_ret;
> +	meta = kvzalloc(meta_size, GFP_KERNEL);
> +	if (!meta)
> +		return ERR_PTR(-ENOMEM);
>   
>   	for (i = 0; i < PCACHE_META_INDEX_MAX; i++) {
> -		meta_addr = (void *)header + (i * meta_max_size);
> +		void *meta_addr = (void *)header + (i * meta_max_size);
> +
>   		if (copy_mc_to_kernel(meta, meta_addr, meta_size)) {
>   			pcache_err("hardware memory error when copy meta");
>   			return ERR_PTR(-EIO);

Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest()

[Li Chen]

Hi Dongsheng, 

 ---- On Mon, 10 Nov 2025 19:18:38 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote --- 
 > Hi Li,
 > 
 >      It seems you sent the same patch again, shoud it be V2 instead?

Apologies for the duplicate patchset. I mistakenly included pcache in that GCC plugin series.
I will send the pcache v2 patchset tomorrow.

Regards,

Li​