Skip to content

Commit ab9517f

Browse files
torvaldstorvalds
committed
Merge tag 'core-debugobjects-2024-01-08' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull debugobject update from Ingo Molnar: - Make tracking object use more robust: it's not safe to access a tracking object after releasing the hashbucket lock. Create a persistent copy for debug printouts instead. * tag 'core-debugobjects-2024-01-08' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: debugobjects: Stop accessing objects after releasing hash bucket lock
2 parents 669d089 + 9bb6362 commit ab9517f

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)