From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f66.google.com ([209.85.160.66]:44737 "EHLO mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752342AbeGBSCS (ORCPT ); Mon, 2 Jul 2018 14:02:18 -0400 Received: by mail-pl0-f66.google.com with SMTP id m16-v6so8298804pls.11 for ; Mon, 02 Jul 2018 11:02:18 -0700 (PDT) Message-ID: <1530554497.16350.2.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: linux-fsdevel@vger.kernel.org, Andrew Morton , Anatoly Trosinenko Date: Mon, 02 Jul 2018 11:01:37 -0700 In-Reply-To: <803590a35221fbf411b2c141419aea3233a6e990.1530294813.git.ernesto.mnd.fernandez@gmail.com> References: <803590a35221fbf411b2c141419aea3233a6e990.1530294813.git.ernesto.mnd.fernandez@gmail.com> 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 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? Thanks, Vyacheslav Dubeyko. > + node = hfs_bnode_find(tree, tree->leaf_head); > + if (IS_ERR(node)) > + return PTR_ERR(node); > + fd->bnode = node; > fd->record = -1; > } > new_node = NULL;