| Message ID | 20221018131432.434167-2-yukuai3@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: fix uaf in bd_link_disk_holder() | expand |
On Tue, Oct 18, 2022 at 09:14:31PM +0800, Yu Kuai wrote: > The return value will be used in later patch to fix uaf for slave_dir > and bd_holder_dir in block layer. Then the user will be incorrect, this is not ok, you should never care if you are the last "put" on an object at all. Hint, what happens right after you call this and get the result? sorry, but NAK. greg k-h
在 2022/10/18 21:00, Greg KH 写道: > On Tue, Oct 18, 2022 at 09:14:31PM +0800, Yu Kuai wrote: >> The return value will be used in later patch to fix uaf for slave_dir >> and bd_holder_dir in block layer. > > Then the user will be incorrect, this is not ok, you should never care > if you are the last "put" on an object at all. Hint, what happens right > after you call this and get the result? > I tried to reset the pointer to NULL in patch 2 to prevent uaf. And the whole kobject_put() and pointer reset is protected by a mutex, the mutex will be used on the reader side before kobject_get as well. So, in fact, I'm protecting them by the mutex... I can bypass it by using another reference anyway. But let's see if anyone has suggestions on the other patch. > sorry, but NAK. I know the best way is too refactor the lifecycle of the problematic bd_holder_dir/slave_dir, however, I gave that up because this seems quite complicated and influence is very huge... Thanks, Kuai > > greg k-h > . >
On Tue, Oct 18, 2022 at 09:12:08PM +0800, Yu Kuai wrote: > > > 在 2022/10/18 21:00, Greg KH 写道: > > On Tue, Oct 18, 2022 at 09:14:31PM +0800, Yu Kuai wrote: > > > The return value will be used in later patch to fix uaf for slave_dir > > > and bd_holder_dir in block layer. > > > > Then the user will be incorrect, this is not ok, you should never care > > if you are the last "put" on an object at all. Hint, what happens right > > after you call this and get the result? > > > > I tried to reset the pointer to NULL in patch 2 to prevent uaf. That is not ok, sorry. > And the > whole kobject_put() and pointer reset is protected by a mutex, the mutex > will be used on the reader side before kobject_get as well. So, in fact, > I'm protecting them by the mutex... Still not ok. You never know who else has a reference on a kobject, that's the point of reference counted objects. > I can bypass it by using another reference anyway. But let's see if > anyone has suggestions on the other patch. > > > sorry, but NAK. > > I know the best way is too refactor the lifecycle of the problematic > bd_holder_dir/slave_dir, however, I gave that up because this seems > quite complicated and influence is very huge... Please fix it up properly, core changes like this should not be needed. thanks, greg k-h
diff --git a/include/linux/kobject.h b/include/linux/kobject.h index 57fb972fea05..f12de6274c51 100644 --- a/include/linux/kobject.h +++ b/include/linux/kobject.h @@ -110,7 +110,7 @@ extern int __must_check kobject_move(struct kobject *, struct kobject *); extern struct kobject *kobject_get(struct kobject *kobj); extern struct kobject * __must_check kobject_get_unless_zero( struct kobject *kobj); -extern void kobject_put(struct kobject *kobj); +extern bool kobject_put(struct kobject *kobj); extern const void *kobject_namespace(struct kobject *kobj); extern void kobject_get_ownership(struct kobject *kobj, diff --git a/lib/kobject.c b/lib/kobject.c index a0b2dbfcfa23..f86c55ae7376 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -711,15 +711,18 @@ static void kobject_release(struct kref *kref) * * Decrement the refcount, and if 0, call kobject_cleanup(). */ -void kobject_put(struct kobject *kobj) +bool kobject_put(struct kobject *kobj) { if (kobj) { if (!kobj->state_initialized) WARN(1, KERN_WARNING "kobject: '%s' (%p): is not initialized, yet kobject_put() is being called.\n", kobject_name(kobj), kobj); - kref_put(&kobj->kref, kobject_release); + if (kref_put(&kobj->kref, kobject_release)) + return true; } + + return false; } EXPORT_SYMBOL(kobject_put);
The return value will be used in later patch to fix uaf for slave_dir and bd_holder_dir in block layer. Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- include/linux/kobject.h | 2 +- lib/kobject.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-)