Re: [Linux-kernel-mentees] [PATCH] hfs, hfsplus: Fix NULL pointer dereference in hfs_find_init()

[Greg Kroah-Hartman]

On Wed, Aug 12, 2020 at 03:13:06AM -0400, Peilin Ye wrote:
> On Wed, Aug 12, 2020 at 09:08:27AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Aug 12, 2020 at 02:55:56AM -0400, Peilin Ye wrote:
> > > Prevent hfs_find_init() from dereferencing `tree` as NULL.
> > > 
> > > Reported-and-tested-by: syzbot+7ca256d0da4af073b2e2@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > Signed-off-by: Peilin Ye <yepeilin.cs@xxxxxxxxx>
> > > ---
> > >  fs/hfs/bfind.c     | 3 +++
> > >  fs/hfsplus/bfind.c | 3 +++
> > >  2 files changed, 6 insertions(+)
> > > 
> > > diff --git a/fs/hfs/bfind.c b/fs/hfs/bfind.c
> > > index 4af318fbda77..880b7ea2c0fc 100644
> > > --- a/fs/hfs/bfind.c
> > > +++ b/fs/hfs/bfind.c
> > > @@ -16,6 +16,9 @@ int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd)
> > >  {
> > >  	void *ptr;
> > >  
> > > +	if (!tree)
> > > +		return -EINVAL;
> > > +
> > >  	fd->tree = tree;
> > >  	fd->bnode = NULL;
> > >  	ptr = kmalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);
> > > diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
> > > index ca2ba8c9f82e..85bef3e44d7a 100644
> > > --- a/fs/hfsplus/bfind.c
> > > +++ b/fs/hfsplus/bfind.c
> > > @@ -16,6 +16,9 @@ int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd)
> > >  {
> > >  	void *ptr;
> > >  
> > > +	if (!tree)
> > > +		return -EINVAL;
> > > +
> > 
> > How can tree ever be NULL in these calls?  Shouldn't that be fixed as
> > the root problem here?
> 
> I see, I will try to figure out what is going on with the reproducer.

That's good to figure out.  Note, your patch might be the correct thing
to do, as that might be an allowed way to call the function.  But in
looking at all the callers, they seem to think they have a valid pointer
at the moment, so perhaps if this check is added, some other root
problem is papered over to be only found later on?

thanks,

greg k-h