From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-184.mta0.migadu.com (out-184.mta0.migadu.com [91.218.175.184]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E04211FDA8E for ; Wed, 5 Nov 2025 01:16:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.184 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762305398; cv=none; b=VL/jKfgzIyg9KJzacTUwu3WdemrdnGV4g3FQMNgITQ4B2fpNRryEd9Ctq/rgHlrB7HeVtDrVZk5r5lw87IUqWa6rwdPvKiymt+9bCFdHImTkHP/wVddgHjrKA4WwPQNhJrMmvQW8m0nvbTOumSQB5vh0NdxlVDkgWDCsccqukIc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762305398; c=relaxed/simple; bh=7RvGY0JpvLJh6bxRUlEGlWcuWjOsuM8XXwnjhADja9c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=hmdQ09XbaTGyICglksWjDG1N0xNKsvV6KQcm4DKwxciG1edA+LdkiWP5GLzbPT3iR5sm7YwJxTMjtvflopBv1GUkxK4G03UjHwPh6BQKQ91d7WS5F3lI3v3n56h9jUN9BR4/LYd3tSgPFq6wNZK0TxNGZxxpDgn3AKz0nHhyTIg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=hPNO8rTN; arc=none smtp.client-ip=91.218.175.184 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="hPNO8rTN" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1762305383; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZX6EkwH7noion1PT5umH7ICWQxPas+RwFp4MAF8onb8=; b=hPNO8rTNx5LYTdF8mN+FyN4cNOfOJ6ZxTET6mjPn2mFbdpMNGhvF6/Rcsq2R1IMzKTty8e 44Nn24qbp9oSE7T/3uXSkPvu1/KjFc+hIqVkB/3mi4/bUcLMPCCY4Ww30yqfBrdpn8lS9V CHLA6UuOai6/3wCrVMRagHxzpZVOL8g= Date: Wed, 5 Nov 2025 09:16:12 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest() To: Li Chen Cc: Zheng Gu , dm-devel , linux-kernel References: <20251030123323.967081-1-me@linux.beauty> <20251030123323.967081-4-me@linux.beauty> <19a3f8ae9db.8a18892d3330798.1127794710232272337@linux.beauty> <1248c78a-c471-496b-b7ac-892ac7335a08@linux.dev> <19a4f153cdf.4d1effd9893048.2344913209212358613@linux.beauty> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Dongsheng Yang In-Reply-To: <19a4f153cdf.4d1effd9893048.2344913209212358613@linux.beauty> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 11/4/2025 9:36 PM, Li Chen 写道: > Hi Dongsheng, > > > ---- On Tue, 04 Nov 2025 14:46:33 +0800 Dongsheng Yang wrote --- > > > > 在 11/1/2025 9:10 PM, Li Chen 写道: > > > Hi Zheng, > > > > > > ---- On Fri, 31 Oct 2025 10:01:23 +0800 Zheng Gu wrote --- > > > >> On Thu, Oct 30, 2025 at 8:36 PM Li Chen wrote:From: Li Chen > > > >> > > > >> 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​ >