From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f196.google.com ([209.85.210.196]:34409 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728490AbeHVXyK (ORCPT ); Wed, 22 Aug 2018 19:54:10 -0400 Received: by mail-pf1-f196.google.com with SMTP id k19-v6so1518138pfi.1 for ; Wed, 22 Aug 2018 13:27:49 -0700 (PDT) Message-ID: <1534969667.2878.8.camel@slavad-ubuntu-14.04> Subject: Re: [PATCH 1/2] hfsplus: prevent crash on exit from failed search From: Viacheslav Dubeyko To: "Ernesto A." =?ISO-8859-1?Q?Fern=E1ndez?= Cc: Andrew Morton , linux-fsdevel@vger.kernel.org, Anatoly Trosinenko Date: Wed, 22 Aug 2018 13:27:47 -0700 In-Reply-To: <20180822181105.ettqvxbgedsdc6kw@eaf> References: <803590a35221fbf411b2c141419aea3233a6e990.1530294813.git.ernesto.mnd.fernandez@gmail.com> <1530554497.16350.2.camel@slavad-ubuntu-14.04> <20180821160224.de4301a45eab98b551823a6c@linux-foundation.org> <20180822181105.ettqvxbgedsdc6kw@eaf> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, 2018-08-22 at 15:11 -0300, Ernesto A. Fernández wrote: > On Tue, Aug 21, 2018 at 04:02:24PM -0700, Andrew Morton wrote: > > On Mon, 02 Jul 2018 11:01:37 -0700 Viacheslav Dubeyko wrote: > > > > > On Fri, 2018-06-29 at 15:34 -0300, Ernesto A. Fernández wrote: > > > > The hfs_find_exit() function expects fd->bnode to be NULL after a > > > > search has failed. The hfs_brec_insert() function may instead set > > > > it to an error-valued pointer. Fix this to prevent a crash. > > > > > > > > Reported-by: Anatoly Trosinenko > > > > Signed-off-by: Ernesto A. Fernández > > > > --- > > > > fs/hfsplus/brec.c | 7 ++++--- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c > > > > index 808f4d8c859c..ed8eacb34452 100644 > > > > --- a/fs/hfsplus/brec.c > > > > +++ b/fs/hfsplus/brec.c > > > > @@ -73,9 +73,10 @@ int hfs_brec_insert(struct hfs_find_data *fd, void *entry, int entry_len) > > > > if (!fd->bnode) { > > > > if (!tree->root) > > > > hfs_btree_inc_height(tree); > > > > - fd->bnode = hfs_bnode_find(tree, tree->leaf_head); > > > > - if (IS_ERR(fd->bnode)) > > > > - return PTR_ERR(fd->bnode); > > > > > > > > > Are you sure that no caller is used this error code? Did you check this? > > > > > > Maybe, it makes sense to extract the error code and to show the error > > > message on the caller side instead of processing the simple NULL? > > > > > > > No response? Could we please get this wrapped up? > > I'm sorry, I thought you had picked this up already. Yes, I did check that > no caller was using this. fd->bnode is always assumed to be NULL on error. > Also, the error code is not lost, it's the return value of the function. OK. Looks reasonable. Reviewed-by: Vyacheslav Dubeyko Thanks, Vyacheslav Dubeyko.