From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from m16.mail.163.com (m16.mail.163.com [220.197.31.4]) (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 D32F030AD1C for ; Tue, 6 Jan 2026 08:12:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=220.197.31.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767687158; cv=none; b=fNdijrpxoXC6kDLa6oG2S91H4ixJMP4h5ciE0c7saeTgKztZUQq5kXkcGad72kqROcpo+FAAwCLSErVSKDkL1aaOXiNiRhcAqxekqRA+8KD+gbi4SvhlP0MRH27zJwR+q2JtA8AmYIh1zsPS8+nKWxYsUi1avBdSgY2GtSWOk28= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767687158; c=relaxed/simple; bh=5/PMLo1LqHww1PE0jh5igMPG/TTTJvDib7m5Rf+g1NI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=UgfmEqKKnQANHHjQ5JaNcabq7khxospfLqJZqUB5BorTOGyeIsxRLh4VCq/2mgGX+mTQ3k+Xnn8w46aQI5YceSG6SN3n+hGOJLCWx+1d48hOub4ur+ktPEtTtUb/elb6aszFkT55Wu1NJjY4EcAYXYRYJZTfcpdJCU5OO4YN/FQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com; spf=pass smtp.mailfrom=163.com; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b=pktH58f0; arc=none smtp.client-ip=220.197.31.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=163.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b="pktH58f0" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:To:From: Content-Type; bh=TSfxlB4XvTuBrg4WL8fT/ZHBwaHKXvehIxTV2YGG3zA=; b=pktH58f0BVCm5ZI0ixtBJshFdVgEG8ZpHwkRQUvn3JDh1yhateMNI3pTFSO7Wn oSnlDbMWhlUVXN9d58NJzNgow3Et2EvzJ81CIlnabam4+uNw1hdxVkZLOQGBBP6n 13j8x/0YswEDTG/EzXtp/sK3ED/p92cTrppvYtC8quzCk= Received: from [192.168.18.185] (unknown []) by gzga-smtp-mtada-g0-1 (Coremail) with SMTP id _____wCHPr64w1xpD0jbEA--.3055S2; Tue, 06 Jan 2026 16:11:43 +0800 (CST) Message-ID: <28f3272c-90bf-48a5-a272-244a0481f51a@163.com> Date: Tue, 6 Jan 2026 16:11:36 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] erofs: Fix state inconsistency when updating fsid/domain_id To: Hongbo Li , xiang@kernel.org, chao@kernel.org Cc: zbestahu@gmail.com, jefflexu@linux.alibaba.com, dhavale@google.com, guochunhai@vivo.com, linux-erofs@lists.ozlabs.org, linux-kernel@vger.kernel.org, Baolin Liu References: <20260106025502.133470-1-liubaolin12138@163.com> From: liubaolin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID:_____wCHPr64w1xpD0jbEA--.3055S2 X-Coremail-Antispam: 1Uf129KBjvJXoW7AFWUGF1fCF1fJryktF47urg_yoW8Kr4UpF Z3K3WFyrZrAr1jgasagr48XF9Y9340y34kK34FqF1kXw15tFn2q3yaqr1jkryfZrZayw40 qFnruwsrWFyYyFDanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07U5sqAUUUUU= X-CM-SenderInfo: xolxutxrol0iasrtmqqrwthudrp/xtbCwh+RXGlcw7+z5wAA3N > Dear Hongbo Li, > > I have reviewed this carefully, and I agree with your point. The old value will eventually be freed in erofs_sb_free(), and keeping it here does not appear to be necessary. Therefore, this patch does not need to be considered further. > > Thank you for your review. > > Dear Gao Xiang, > > Thank you for your review as well. > > Best regards, > Baolin Liu > > 在 2026/1/6 11:30, Hongbo Li 写道: > Hi, > > On 2026/1/6 10:55, Baolin Liu wrote: >> From: Baolin Liu >> >> When updating fsid or domain_id, the code frees the old pointer before >> allocating a new one. If allocation fails, the pointer becomes NULL >> while the old value is already freed, causing state inconsistency. >> >> Fix by allocating the new value first, and only freeing the old value >> on success. >> >> Signed-off-by: Baolin Liu >> --- >>   fs/erofs/super.c | 18 ++++++++++++------ >>   1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/fs/erofs/super.c b/fs/erofs/super.c >> index 937a215f626c..6e083d7e634c 100644 >> --- a/fs/erofs/super.c >> +++ b/fs/erofs/super.c >> @@ -509,16 +509,22 @@ static int erofs_fc_parse_param(struct >> fs_context *fc, >>           break; >>   #ifdef CONFIG_EROFS_FS_ONDEMAND >>       case Opt_fsid: >> -        kfree(sbi->fsid); >> -        sbi->fsid = kstrdup(param->string, GFP_KERNEL); >> -        if (!sbi->fsid) >> +        char *new_fsid; >> + >> +        new_fsid = kstrdup(param->string, GFP_KERNEL); > > May be there is no need to keep the old pointer. Because > 1) The fsid/domain_id is ignored in reconfiguration. > 2) Even if memory allocation fails when the user first mounts with multi > fsid/domain_id options (like -o fsid=xxx1,fsid=xxx2), the old fsid > pointer would also need to be released in cleanup procedure. > > so am I right? > > Thanks, > Hongbo > >> +        if (!new_fsid) >>               return -ENOMEM; >> +        kfree(sbi->fsid); >> +        sbi->fsid = new_fsid; >>           break; >>       case Opt_domain_id: >> -        kfree(sbi->domain_id); >> -        sbi->domain_id = kstrdup(param->string, GFP_KERNEL); >> -        if (!sbi->domain_id) >> +        char *new_domain_id; >> + >> +        new_domain_id = kstrdup(param->string, GFP_KERNEL); >> +        if (!new_domain_id) >>               return -ENOMEM; >> +        kfree(sbi->domain_id); >> +        sbi->domain_id = new_domain_id; >>           break; >>   #else >>       case Opt_fsid: