Skip to content

Commit 9bb6362

Browse files
ahajdaKAGA-KOKO
ahajda
authored and
KAGA-KOKO
committed
debugobjects: Stop accessing objects after releasing hash bucket lock
After release of the hashbucket lock the tracking object can be modified or freed by a concurrent thread. Using it in such a case is error prone, even for printing the object state: 1. T1 tries to deactivate destroyed object, debugobjects detects it, hash bucket lock is released. 2. T2 preempts T1 and frees the tracking object. 3. The freed tracking object is allocated and initialized for a different to be tracked kernel object. 4. T1 resumes and reports error for wrong kernel object. Create a local copy of the tracking object before releasing the hash bucket lock and use the local copy for reporting and fixups to prevent this. Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Link: https://lore.kernel.org/r/20231025-debugobjects_fix-v3-1-2bc3bf7084c2@intel.com
1 parent 98b1cc8 commit 9bb6362

1 file changed

Lines changed: 78 additions & 122 deletions

File tree

‎lib/debugobjects.c‎

Lines changed: 78 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -620,9 +620,8 @@ static void debug_objects_fill_pool(void)
620620
static void
621621
__debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack)
622622
{
623-
enum debug_obj_state state;
623+
struct debug_obj *obj, o;
624624
struct debug_bucket *db;
625-
struct debug_obj *obj;
626625
unsigned long flags;
627626

628627
debug_objects_fill_pool();
@@ -643,24 +642,18 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack
643642
case ODEBUG_STATE_INIT:
644643
case ODEBUG_STATE_INACTIVE:
645644
obj->state = ODEBUG_STATE_INIT;
646-
break;
647-
648-
case ODEBUG_STATE_ACTIVE:
649-
state = obj->state;
650-
raw_spin_unlock_irqrestore(&db->lock, flags);
651-
debug_print_object(obj, "init");
652-
debug_object_fixup(descr->fixup_init, addr, state);
653-
return;
654-
655-
case ODEBUG_STATE_DESTROYED:
656645
raw_spin_unlock_irqrestore(&db->lock, flags);
657-
debug_print_object(obj, "init");
658646
return;
659647
default:
660648
break;
661649
}
662650

651+
o = *obj;
663652
raw_spin_unlock_irqrestore(&db->lock, flags);
653+
debug_print_object(&o, "init");
654+
655+
if (o.state == ODEBUG_STATE_ACTIVE)
656+
debug_object_fixup(descr->fixup_init, addr, o.state);
664657
}
665658

666659
/**
@@ -701,11 +694,9 @@ EXPORT_SYMBOL_GPL(debug_object_init_on_stack);
701694
int debug_object_activate(void *addr, const struct debug_obj_descr *descr)
702695
{
703696
struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr };
704-
enum debug_obj_state state;
705697
struct debug_bucket *db;
706698
struct debug_obj *obj;
707699
unsigned long flags;
708-
int ret;
709700

710701
if (!debug_objects_enabled)
711702
return 0;
@@ -717,49 +708,38 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr)
717708
raw_spin_lock_irqsave(&db->lock, flags);
718709

719710
obj = lookup_object_or_alloc(addr, db, descr, false, true);
720-
if (likely(!IS_ERR_OR_NULL(obj))) {
721-
bool print_object = false;
722-
711+
if (unlikely(!obj)) {
712+
raw_spin_unlock_irqrestore(&db->lock, flags);
713+
debug_objects_oom();
714+
return 0;
715+
} else if (likely(!IS_ERR(obj))) {
723716
switch (obj->state) {
724-
case ODEBUG_STATE_INIT:
725-
case ODEBUG_STATE_INACTIVE:
726-
obj->state = ODEBUG_STATE_ACTIVE;
727-
ret = 0;
728-
break;
729-
730717
case ODEBUG_STATE_ACTIVE:
731-
state = obj->state;
732-
raw_spin_unlock_irqrestore(&db->lock, flags);
733-
debug_print_object(obj, "activate");
734-
ret = debug_object_fixup(descr->fixup_activate, addr, state);
735-
return ret ? 0 : -EINVAL;
736-
737718
case ODEBUG_STATE_DESTROYED:
738-
print_object = true;
739-
ret = -EINVAL;
719+
o = *obj;
740720
break;
721+
case ODEBUG_STATE_INIT:
722+
case ODEBUG_STATE_INACTIVE:
723+
obj->state = ODEBUG_STATE_ACTIVE;
724+
fallthrough;
741725
default:
742-
ret = 0;
743-
break;
726+
raw_spin_unlock_irqrestore(&db->lock, flags);
727+
return 0;
744728
}
745-
raw_spin_unlock_irqrestore(&db->lock, flags);
746-
if (print_object)
747-
debug_print_object(obj, "activate");
748-
return ret;
749729
}
750730

751731
raw_spin_unlock_irqrestore(&db->lock, flags);
732+
debug_print_object(&o, "activate");
752733

753-
/* If NULL the allocation has hit OOM */
754-
if (!obj) {
755-
debug_objects_oom();
756-
return 0;
734+
switch (o.state) {
735+
case ODEBUG_STATE_ACTIVE:
736+
case ODEBUG_STATE_NOTAVAILABLE:
737+
if (debug_object_fixup(descr->fixup_activate, addr, o.state))
738+
return 0;
739+
fallthrough;
740+
default:
741+
return -EINVAL;
757742
}
758-
759-
/* Object is neither static nor tracked. It's not initialized */
760-
debug_print_object(&o, "activate");
761-
ret = debug_object_fixup(descr->fixup_activate, addr, ODEBUG_STATE_NOTAVAILABLE);
762-
return ret ? 0 : -EINVAL;
763743
}
764744
EXPORT_SYMBOL_GPL(debug_object_activate);
765745

@@ -770,10 +750,10 @@ EXPORT_SYMBOL_GPL(debug_object_activate);
770750
*/
771751
void debug_object_deactivate(void *addr, const struct debug_obj_descr *descr)
772752
{
753+
struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr };
773754
struct debug_bucket *db;
774755
struct debug_obj *obj;
775756
unsigned long flags;
776-
bool print_object = false;
777757

778758
if (!debug_objects_enabled)
779759
return;
@@ -785,33 +765,24 @@ void debug_object_deactivate(void *addr, const struct debug_obj_descr *descr)
785765
obj = lookup_object(addr, db);
786766
if (obj) {
787767
switch (obj->state) {
768+
case ODEBUG_STATE_DESTROYED:
769+
break;
788770
case ODEBUG_STATE_INIT:
789771
case ODEBUG_STATE_INACTIVE:
790772
case ODEBUG_STATE_ACTIVE:
791-
if (!obj->astate)
792-
obj->state = ODEBUG_STATE_INACTIVE;
793-
else
794-
print_object = true;
795-
break;
796-
797-
case ODEBUG_STATE_DESTROYED:
798-
print_object = true;
799-
break;
773+
if (obj->astate)
774+
break;
775+
obj->state = ODEBUG_STATE_INACTIVE;
776+
fallthrough;
800777
default:
801-
break;
778+
raw_spin_unlock_irqrestore(&db->lock, flags);
779+
return;
802780
}
781+
o = *obj;
803782
}
804783

805784
raw_spin_unlock_irqrestore(&db->lock, flags);
806-
if (!obj) {
807-
struct debug_obj o = { .object = addr,
808-
.state = ODEBUG_STATE_NOTAVAILABLE,
809-
.descr = descr };
810-
811-
debug_print_object(&o, "deactivate");
812-
} else if (print_object) {
813-
debug_print_object(obj, "deactivate");
814-
}
785+
debug_print_object(&o, "deactivate");
815786
}
816787
EXPORT_SYMBOL_GPL(debug_object_deactivate);
817788

@@ -822,11 +793,9 @@ EXPORT_SYMBOL_GPL(debug_object_deactivate);
822793
*/
823794
void debug_object_destroy(void *addr, const struct debug_obj_descr *descr)
824795
{
825-
enum debug_obj_state state;
796+
struct debug_obj *obj, o;
826797
struct debug_bucket *db;
827-
struct debug_obj *obj;
828798
unsigned long flags;
829-
bool print_object = false;
830799

831800
if (!debug_objects_enabled)
832801
return;
@@ -836,32 +805,31 @@ void debug_object_destroy(void *addr, const struct debug_obj_descr *descr)
836805
raw_spin_lock_irqsave(&db->lock, flags);
837806

838807
obj = lookup_object(addr, db);
839-
if (!obj)
840-
goto out_unlock;
808+
if (!obj) {
809+
raw_spin_unlock_irqrestore(&db->lock, flags);
810+
return;
811+
}
841812

842813
switch (obj->state) {
814+
case ODEBUG_STATE_ACTIVE:
815+
case ODEBUG_STATE_DESTROYED:
816+
break;
843817
case ODEBUG_STATE_NONE:
844818
case ODEBUG_STATE_INIT:
845819
case ODEBUG_STATE_INACTIVE:
846820
obj->state = ODEBUG_STATE_DESTROYED;
847-
break;
848-
case ODEBUG_STATE_ACTIVE:
849-
state = obj->state;
821+
fallthrough;
822+
default:
850823
raw_spin_unlock_irqrestore(&db->lock, flags);
851-
debug_print_object(obj, "destroy");
852-
debug_object_fixup(descr->fixup_destroy, addr, state);
853824
return;
854-
855-
case ODEBUG_STATE_DESTROYED:
856-
print_object = true;
857-
break;
858-
default:
859-
break;
860825
}
861-
out_unlock:
826+
827+
o = *obj;
862828
raw_spin_unlock_irqrestore(&db->lock, flags);
863-
if (print_object)
864-
debug_print_object(obj, "destroy");
829+
debug_print_object(&o, "destroy");
830+
831+
if (o.state == ODEBUG_STATE_ACTIVE)
832+
debug_object_fixup(descr->fixup_destroy, addr, o.state);
865833
}
866834
EXPORT_SYMBOL_GPL(debug_object_destroy);
867835

@@ -872,9 +840,8 @@ EXPORT_SYMBOL_GPL(debug_object_destroy);
872840
*/
873841
void debug_object_free(void *addr, const struct debug_obj_descr *descr)
874842
{
875-
enum debug_obj_state state;
843+
struct debug_obj *obj, o;
876844
struct debug_bucket *db;
877-
struct debug_obj *obj;
878845
unsigned long flags;
879846

880847
if (!debug_objects_enabled)
@@ -885,24 +852,26 @@ void debug_object_free(void *addr, const struct debug_obj_descr *descr)
885852
raw_spin_lock_irqsave(&db->lock, flags);
886853

887854
obj = lookup_object(addr, db);
888-
if (!obj)
889-
goto out_unlock;
855+
if (!obj) {
856+
raw_spin_unlock_irqrestore(&db->lock, flags);
857+
return;
858+
}
890859

891860
switch (obj->state) {
892861
case ODEBUG_STATE_ACTIVE:
893-
state = obj->state;
894-
raw_spin_unlock_irqrestore(&db->lock, flags);
895-
debug_print_object(obj, "free");
896-
debug_object_fixup(descr->fixup_free, addr, state);
897-
return;
862+
break;
898863
default:
899864
hlist_del(&obj->node);
900865
raw_spin_unlock_irqrestore(&db->lock, flags);
901866
free_object(obj);
902867
return;
903868
}
904-
out_unlock:
869+
870+
o = *obj;
905871
raw_spin_unlock_irqrestore(&db->lock, flags);
872+
debug_print_object(&o, "free");
873+
874+
debug_object_fixup(descr->fixup_free, addr, o.state);
906875
}
907876
EXPORT_SYMBOL_GPL(debug_object_free);
908877

@@ -954,10 +923,10 @@ void
954923
debug_object_active_state(void *addr, const struct debug_obj_descr *descr,
955924
unsigned int expect, unsigned int next)
956925
{
926+
struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr };
957927
struct debug_bucket *db;
958928
struct debug_obj *obj;
959929
unsigned long flags;
960-
bool print_object = false;
961930

962931
if (!debug_objects_enabled)
963932
return;
@@ -970,41 +939,30 @@ debug_object_active_state(void *addr, const struct debug_obj_descr *descr,
970939
if (obj) {
971940
switch (obj->state) {
972941
case ODEBUG_STATE_ACTIVE:
973-
if (obj->astate == expect)
974-
obj->astate = next;
975-
else
976-
print_object = true;
977-
break;
978-
942+
if (obj->astate != expect)
943+
break;
944+
obj->astate = next;
945+
raw_spin_unlock_irqrestore(&db->lock, flags);
946+
return;
979947
default:
980-
print_object = true;
981948
break;
982949
}
950+
o = *obj;
983951
}
984952

985953
raw_spin_unlock_irqrestore(&db->lock, flags);
986-
if (!obj) {
987-
struct debug_obj o = { .object = addr,
988-
.state = ODEBUG_STATE_NOTAVAILABLE,
989-
.descr = descr };
990-
991-
debug_print_object(&o, "active_state");
992-
} else if (print_object) {
993-
debug_print_object(obj, "active_state");
994-
}
954+
debug_print_object(&o, "active_state");
995955
}
996956
EXPORT_SYMBOL_GPL(debug_object_active_state);
997957

998958
#ifdef CONFIG_DEBUG_OBJECTS_FREE
999959
static void __debug_check_no_obj_freed(const void *address, unsigned long size)
1000960
{
1001961
unsigned long flags, oaddr, saddr, eaddr, paddr, chunks;
1002-
const struct debug_obj_descr *descr;
1003-
enum debug_obj_state state;
962+
int cnt, objs_checked = 0;
963+
struct debug_obj *obj, o;
1004964
struct debug_bucket *db;
1005965
struct hlist_node *tmp;
1006-
struct debug_obj *obj;
1007-
int cnt, objs_checked = 0;
1008966

1009967
saddr = (unsigned long) address;
1010968
eaddr = saddr + size;
@@ -1026,12 +984,10 @@ static void __debug_check_no_obj_freed(const void *address, unsigned long size)
1026984

1027985
switch (obj->state) {
1028986
case ODEBUG_STATE_ACTIVE:
1029-
descr = obj->descr;
1030-
state = obj->state;
987+
o = *obj;
1031988
raw_spin_unlock_irqrestore(&db->lock, flags);
1032-
debug_print_object(obj, "free");
1033-
debug_object_fixup(descr->fixup_free,
1034-
(void *) oaddr, state);
989+
debug_print_object(&o, "free");
990+
debug_object_fixup(o.descr->fixup_free, (void *)oaddr, o.state);
1035991
goto repeat;
1036992
default:
1037993
hlist_del(&obj->node);

0 commit comments

Comments
 (0)