Re: [PATCH 7/8] cachefiles: Fix NULL pointer dereference in object->file

[Zizhi Wo <wozizhi@xxxxxxxxxx>]

在 2024/10/10 22:52, David Howells 写道:
> Zizhi Wo <wozizhi@xxxxxxxxxx> wrote:
>
> > 在 2024/10/10 19:26, David Howells 写道:
> > > Zizhi Wo <wozizhi@xxxxxxxxxx> wrote:
> > >
> > > > +	spin_lock(&object->lock);
> > > >    	if (object->file) {
> > > >    		fput(object->file);
> > > >    		object->file = NULL;
> > > >    	}
> > > > +	spin_unlock(&object->lock);
> > > I would suggest stashing the file pointer in a local var and then doing the
> > > fput() outside of the locks.
> > > David
> >
> > If fput() is executed outside the lock, I am currently unsure how to
> > guarantee that file in __cachefiles_write() does not trigger null
> > pointer dereference...
>
> I'm not sure why there's a problem here.  I was thinking along the lines of:
>
> 	struct file *tmp;
> 	spin_lock(&object->lock);
>   	tmp = object->file)
> 	object->file = NULL;
> 	spin_unlock(&object->lock);
> 	if (tmp)
> 		fput(tmp);
>
> Note that fput() may defer the actual work if the counter hits zero, so the
> cleanup may not happen inside the lock; further, the cleanup done by __fput()
> may sleep.
>
> David
Oh, I see what you mean. I will sort it out and issue the second patch
as soon as possible.

Thanks,
Zizhi Wo