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

From: Zizhi Wo
Date: Thu Oct 10 2024 - 21:32:10 EST




在 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