From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C62DA1DE4FB for ; Thu, 11 Dec 2025 23:13:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765494788; cv=none; b=JkQkLUpNrpSXdYh2C4VLOmvDv+o9+rLaRf881bTRj+t86pE7JW0qmbhMIZK++KZoOKlzJPsG2BoLOStaVCmvoP/VlzydSyh4M6wdYrlLnC4TWdrUcOsNh7tIL97xX/MFo80igORwCtzufjBmW/vnPnDcSTV8eqSJzp1J+lzHbdE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765494788; c=relaxed/simple; bh=+SItKWheGOCfJZiVx/abGIZN0q7nJm9pgeN1VExwXjs=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=kOPQKuUHqceLZ18jUgmTTbhuW6uvrp33w4nGIkkZAVUaboUdqThkr5OKDA57TI0bTnwOU/cKiy0jXMmEjDHvZJ2zz+o8UJHNBh9tFcOi9V81E1hvDgB+o6jdUn9upAFSFXNyj+TN71ZhjbLsDh6Vgh1inITuwIc1T0X8ucgrsQg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dubeyko.com; spf=pass smtp.mailfrom=dubeyko.com; dkim=pass (2048-bit key) header.d=dubeyko-com.20230601.gappssmtp.com header.i=@dubeyko-com.20230601.gappssmtp.com header.b=r9ilj/ji; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dubeyko.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=dubeyko.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=dubeyko-com.20230601.gappssmtp.com header.i=@dubeyko-com.20230601.gappssmtp.com header.b="r9ilj/ji" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2956d816c10so7707605ad.1 for ; Thu, 11 Dec 2025 15:13:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dubeyko-com.20230601.gappssmtp.com; s=20230601; t=1765494786; x=1766099586; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=ep1Ckut8sp7EgQ95W6RLTx5bAu00UGPglt77ENNDVlQ=; b=r9ilj/jiUoZipAyK09Qg3ZpJIc8Y77C0F1+yykEqhM6+6BsE2IQhHONMb0oaYcwn/N EwpOkL5IrT+IKZFbWNKBXD+XHzmn+QPxVWt0cE22Eb+B1kfUwDHYCW2q1cWeS0T+IDE+ 5JBiSsqDABl4YonRWhVKYwYnLf/Mgy93+UjtfJ/SyYxbLvrmL3xUTPhFThC0uLonYs7F fjWeJQz1nxH3yWG0GegCHabAwG630S4l3ezy7ygxVoqCJ2R5OIaKloYXQEksM54ceqZx wIa0Zxkb1kkvuM1hv5YDJipBdU1fwjJkts0Audzn+R5kNEBaX6C/b+h/Vorv6bZmk7h7 E5nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765494786; x=1766099586; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ep1Ckut8sp7EgQ95W6RLTx5bAu00UGPglt77ENNDVlQ=; b=iZCFwshqE5hPuA8P5VT1BAw58x+di8y0jERaSL0OW2T5GMhmRGZtmr7qBRg/HcUi7P EecvnVxqlx5YMgsSxJnggbq3PdeF/mrcrBYt896GbmF2rBtZZG1XuDFozZ6gy/aYV7ls Fe8/CRVwrigKoiFBdaWZU6N0MinnNEwaCxpsY9blV3GBFGIPpHr+AuW8HlzftSeVp+mm rul3rkcR7sDVtZZbIP1cO3fWSAOGm4vsEduDfjW5hfjwQMThRNLD1Yqoqg9IMalurYFZ QpOoGw3BX9Y15/noYKTsGkLd3/R7TvvZ5qQlnkUHnPN+eCdsInB6gnFEFgpD8UcEL86e nKGA== X-Forwarded-Encrypted: i=1; AJvYcCVxFeaOvO7bhVzypVC3Xfv+q7LoGAKiQciQlFU8zBQrYILXkTGpR6VIWSJjfNn4s/OUO5356LSyQjhalT8=@vger.kernel.org X-Gm-Message-State: AOJu0YyZuh0ImRA+v1tNs6gLKTLix40f9h4IU3LF/iSYgpc1CWlTXdYa bAhY1MWIi8UplQKqRve0/sKQNlATVKnvytOFmInFNSOzs5akC1Mgyj/KuAoFvKU1zQU= X-Gm-Gg: AY/fxX6NTp53gVvXtwID2zckOP/EbyfjP/mrJaEmRkepIu2zjm8VDCteCd6QlN6uCSp VXzxjh3EOnoH2BIf3lgOx1Ece5H113QAzfT7tu9zZ2ec/ztzhtRMVkCNGXL6PvQoevRiP0IFV8h lEmoEPGdAsl9h+9hnDI3jNvetPA77luKC1anb8PB+hrvhMKOId1Fsl8gSFeqbM7bE/pYMsNAqji S39EVf9+N0+ABTkEQHq+pYY/bUfuiq74jDAHtS7NCHRwN/fT+VM2agN0KtKlXaFR5s7yGqDYPjm KNMzta8y+WNh6k76JYK2/tiQsukQGaXG+MSYbxJ5YzVYEhjAynmIvXhBPQ2rZMz/IlMq7CTu56r 3NXSKopFCYdID7zuPynrWMOACW8AsQg+0Ks7cBxfjOVBEQikjukLm8h4oWT3j9H7zkmP/t81HyL GtsPUi6fMfA0AMQCs7lnvjF4DwpelNrXAkUdJR+wxDj3c= X-Google-Smtp-Source: AGHT+IEAuOQpJuBcBVuPvr8tINKODigjiIkgzbINCo9pRxRax9QFKpjeA1yhbFOKEpZ2X9QT0bqzrw== X-Received: by 2002:a17:903:1250:b0:295:6122:5c42 with SMTP id d9443c01a7336-29f23b6f3fbmr2690415ad.24.1765494786058; Thu, 11 Dec 2025 15:13:06 -0800 (PST) Received: from [172.16.2.132] (fs98a5732b.tkyc510.ap.nuro.jp. [152.165.115.43]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34abe3ba59bsm25923a91.7.2025.12.11.15.13.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Dec 2025 15:13:05 -0800 (PST) Message-ID: <3a143f53da945f7bad35aaff7bb40b1b6255d5ba.camel@dubeyko.com> Subject: Re: [PATCH] HFS: btree: fix missing error check after hfs_bnode_find() From: Viacheslav Dubeyko To: Haotian Zhang , glaubitz@physik.fu-berlin.de, frank.li@vivo.com Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Date: Thu, 11 Dec 2025 15:13:01 -0800 In-Reply-To: <20251209021401.1854-1-vulab@iscas.ac.cn> References: <20251209021401.1854-1-vulab@iscas.ac.cn> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.1 (by Flathub.org) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Tue, 2025-12-09 at 10:14 +0800, Haotian Zhang wrote: > In hfs_brec_insert() and hfs_brec_update_parent(), hfs_bnode_find() > may return ERR_PTR() on failure, but the result was used without > checking, risking NULL pointer dereference or invalid pointer usage. >=20 > Add IS_ERR() checks after these calls and return PTR_ERR() > on error. >=20 > Signed-off-by: Haotian Zhang > --- > =C2=A0fs/hfs/brec.c | 4 ++++ > =C2=A01 file changed, 4 insertions(+) >=20 > diff --git a/fs/hfs/brec.c b/fs/hfs/brec.c > index e49a141c87e5..afa1840a4847 100644 > --- a/fs/hfs/brec.c > +++ b/fs/hfs/brec.c > @@ -149,6 +149,8 @@ int hfs_brec_insert(struct hfs_find_data *fd, > void *entry, int entry_len) > =C2=A0 new_node->parent =3D tree->root; > =C2=A0 } > =C2=A0 fd->bnode =3D hfs_bnode_find(tree, new_node->parent); > + if (IS_ERR(fd->bnode)) > + return PTR_ERR(fd->bnode); > =C2=A0 > =C2=A0 /* create index data entry */ > =C2=A0 cnid =3D cpu_to_be32(new_node->this); > @@ -449,6 +451,8 @@ static int hfs_brec_update_parent(struct > hfs_find_data *fd) > =C2=A0 new_node->parent =3D tree->root; > =C2=A0 } > =C2=A0 fd->bnode =3D hfs_bnode_find(tree, new_node->parent); > + if (IS_ERR(fd->bnode)) > + return PTR_ERR(fd->bnode); > =C2=A0 /* create index key and entry */ > =C2=A0 hfs_bnode_read_key(new_node, fd->search_key, 14); > =C2=A0 cnid =3D cpu_to_be32(new_node->this); Frankly speaking, I am not sure that we need to add this check. Because, we are trying to find the parent node that already has been found in above logic of the method. So, we should have the parent node available. Potentially, logic could work in wrong way, but we should already have a reported bug already. Even if this check makes sense, then we cannot simply return the error code here. If you check the following logic, then you can see that we call hfs_bnode_put() for the new node. So, if this check doesn't do this in the case of error, then we create the leak here. Have you ever reproduced the issue that you are trying to fix? Thanks, Slava.