From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f65.google.com ([209.85.160.65]:35543 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387534AbeGLXHt (ORCPT ); Thu, 12 Jul 2018 19:07:49 -0400 Received: by mail-pl0-f65.google.com with SMTP id k1-v6so11290796plt.2 for ; Thu, 12 Jul 2018 15:56:07 -0700 (PDT) Message-ID: <1531436133.22955.4.camel@slavad-ubuntu-14.04> Subject: Re: [PATCH] hfsplus: fix NULL dereference in hfsplus_lookup() From: Viacheslav Dubeyko To: "Ernesto A." =?ISO-8859-1?Q?Fern=E1ndez?= Cc: linux-fsdevel@vger.kernel.org, Andrew Morton , "Xu, Wen" Date: Thu, 12 Jul 2018 15:55:33 -0700 In-Reply-To: <20180712215344.q44dyrhymm4ajkao@eaf> References: <20180712215344.q44dyrhymm4ajkao@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 Thu, 2018-07-12 at 18:53 -0300, Ernesto A. Fernández wrote: > Check that the hidden directory is not NULL before using it, instead of > after. > > Reported-by: Wen Xu > Signed-off-by: Ernesto A. Fernández > --- It's really hard to understand this simple patch. I believe it makes sense to rework the patch slightly with the goal to make it more clear. Also, it will be great to add a short comment in the code to explain what's wrong. I think it makes sense to split this long check condition on something more clear, simple and elegant. Thanks, Vyacheslav Dubeyko. > fs/hfsplus/dir.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c > index b5254378f011..cd017d7dbdfa 100644 > --- a/fs/hfsplus/dir.c > +++ b/fs/hfsplus/dir.c > @@ -78,13 +78,13 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry, > cpu_to_be32(HFSP_HARDLINK_TYPE) && > entry.file.user_info.fdCreator == > cpu_to_be32(HFSP_HFSPLUS_CREATOR) && > + HFSPLUS_SB(sb)->hidden_dir && > (entry.file.create_date == > HFSPLUS_I(HFSPLUS_SB(sb)->hidden_dir)-> > create_date || > entry.file.create_date == > HFSPLUS_I(d_inode(sb->s_root))-> > - create_date) && > - HFSPLUS_SB(sb)->hidden_dir) { > + create_date)) { > struct qstr str; > char name[32]; >