Fix NULL pointer dereference in mp_obj_get_type #17944 #17948

MaksimFeng · 2025-08-17T20:44:09Z

Summary

py/obj:Against null during list.sort,add testcase.
If a list's comparison function calls array.clear() and gc.collect() during sort, an element slot can
become MP_OBJ_NULL. Guard mp_obj_get_type() to avoid a NULL dereference.

Closes: #17941

Testing

Testing on unix port.

codecov · 2025-08-17T20:53:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (1588c45) to head (0d8c36a).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #17948   +/-   ##
=======================================
  Coverage   98.38%   98.39%           
=======================================
  Files         171      171           
  Lines       22297    22304    +7     
=======================================
+ Hits        21938    21945    +7     
  Misses        359      359

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-08-17T20:53:16Z

Code size report:

   bare-arm:   +96 +0.169% 
minimal x86:  +207 +0.110% 
   unix x64:  +216 +0.025% standard
      stm32:  +128 +0.032% PYBV10
     mimxrt:  +120 +0.032% TEENSY40
        rp2:  +112 +0.012% RPI_PICO_W
       samd:  +128 +0.047% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:  +122 +0.027% VIRT_RV32

agatti · 2025-08-18T01:54:04Z

py/obj.c

@@ -57,6 +57,9 @@ const mp_obj_type_t *MICROPY_WRAP_MP_OBJ_GET_TYPE(mp_obj_get_type)(mp_const_obj_
    #if MICROPY_OBJ_IMMEDIATE_OBJS && MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_A

    if (mp_obj_is_obj(o_in)) {
+        if (o_in == MP_OBJ_NULL) {


I believe this should also be applied to the other instance of mp_obj_is_obj later in the function, and to the final ->type dereference at the end of the function.

If I understood this correctly, this only fixes the issue for MICROPY_OBJ_REPR_A, doesn't it?.

Thanks for the review — you’re right on both points.
My original guard lived only in the MICROPY_OBJ_REPR_A/mp_obj_is_obj() branch, so it would miss other branches and other representations. I’ve moved the MP_OBJ_NULL check to the top of mp_obj_get_type(), before any branching.
To address the crash at its source without changing list layout, I implemented a mutation check in objlist.c. Pass this check to mp_quicksort and check after any call that can run Python code (key function, comparator) and before swaps. If the pointer or length changed, raise ValueError("list modified during sort").

I see. Considering that your change is affecting all ports you may want to move the test from tests/ports/unix to tests/basics instead. If you don't have a board for each port involved to test this just ask, folks here can help you out :)

Also, there's a non-trivial size increase on all ports caused by this change, see the comment for py/objlist.c below to see whether things can be made shorter.

This check should not be needed, there should be no way for MP_OBJ_NULL to get here (at least, if the mutate-while-sorting issue is solved).

Thank you so much for reviewing the code. There're indeed some duplicate checks and I've removed them. Please help me test it on different boards. Currently, I don't have an extra board with me.

agatti · 2025-08-18T13:28:27Z

py/objlist.c

@@ -41,6 +41,11 @@ static mp_obj_t list_pop(size_t n_args, const mp_obj_t *args);

 /******************************************************************************/
 /* list                                                                       */
+static inline void list_sort_mutation_check(mp_obj_list_t *self, mp_obj_t *orig_items, size_t orig_len){
+    if (self->items != orig_items || self->len != orig_len){
+        mp_raise_ValueError(MP_ERROR_TEXT("list modified during sort."));


Error messages shouldn't have a trailing period (I've got bitten by this as well in the past).

agatti · 2025-08-18T13:31:30Z

py/objlist.c

    mp_cstack_check();
    while (head < tail) {
        mp_obj_t *h = head - 1;
        mp_obj_t *t = tail;
        mp_obj_t v = key_fn == MP_OBJ_NULL ? tail[0] : mp_call_function_1(key_fn, tail[0]); // get pivot using key_fn
+        list_sort_mutation_check(self, orig_items, orig_len);


This can probably be removed, as I think the execution flow should always pass through the call to list_sort_mutation_check at line 296.

agatti · 2025-08-18T13:32:01Z

py/objlist.c

            mp_obj_t x = h[0];
            h[0] = t[0];
            t[0] = x;
        }
+        list_sort_mutation_check(self, orig_items, orig_len);


This can probably be removed too, at this point the list should have been checked by the call to list_sort_mutation_check on line 306, I think.

agatti · 2025-08-18T13:35:02Z

py/objlist.c

            } while (h < t && mp_binary_op(MP_BINARY_OP_LESS, key_fn == MP_OBJ_NULL ? h[0] : mp_call_function_1(key_fn, h[0]), v) == binop_less_result);
-            do {--t;
+            list_sort_mutation_check(self, orig_items, orig_len);


This can probably be removed too, as I think execution should pass through line 301 anyway.

dpgeorge · 2025-08-19T00:06:12Z

Thanks for the contribution.

For this to be considered for merging, it needs to be as minimal as possible in terms of code size increase (see above "Code size report", which will change as you push to this PR).

agatti · 2025-08-19T13:17:59Z

@MaksimFeng For the unix/minimal test error you see, said configuration doesn't have the garbage collector compiled in - apologies for missing that during the first review pass.

To work around that, relevant module inclusions that may fail can be wrapped like this:

try:
    import gc
except ImportError:
    print("SKIP")
    raise SystemExit

Existing documentation on writing tests can be a bit scarce, but there's #17301 that aims to flesh that out. You may want to scan those changes as they may clarify a few things.

Edit: the same thing makes the Zephyr test fail too? I thought that had the garbage collector enabled, go figure.

Closes: micropython#17941 Signed-off-by: Kai <maksimfengk@outlook.com>

MaksimFeng · 2025-08-19T17:04:11Z

@agatti, thank you for the reminder and the supplementary document. It was very helpful. It appears the Zephyr test failure was caused by the gc import....

agatti reviewed Aug 18, 2025

View reviewed changes

MaksimFeng force-pushed the bug/fix_gc branch 2 times, most recently from 62b1794 to cc02711 Compare August 18, 2025 11:33

agatti reviewed Aug 18, 2025

View reviewed changes

dpgeorge added the py-core Relates to py/ directory in source label Aug 19, 2025

MaksimFeng force-pushed the bug/fix_gc branch from cc02711 to e01d50d Compare August 19, 2025 12:56

MaksimFeng force-pushed the bug/fix_gc branch from e01d50d to 702995d Compare August 19, 2025 14:22

py/objlist: Prevent crash when list mutated during sort.

0d8c36a

Closes: micropython#17941 Signed-off-by: Kai <maksimfengk@outlook.com>

MaksimFeng force-pushed the bug/fix_gc branch from 702995d to 0d8c36a Compare August 19, 2025 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix NULL pointer dereference in mp_obj_get_type #17944 #17948

Fix NULL pointer dereference in mp_obj_get_type #17944 #17948

Uh oh!

MaksimFeng commented Aug 17, 2025

Uh oh!

codecov bot commented Aug 17, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 17, 2025 •

edited

Loading

Uh oh!

agatti Aug 18, 2025 •

edited

Loading

Uh oh!

MaksimFeng Aug 18, 2025

Uh oh!

agatti Aug 18, 2025

Uh oh!

dpgeorge Aug 19, 2025

Uh oh!

MaksimFeng Aug 19, 2025

Uh oh!

agatti Aug 18, 2025

Uh oh!

agatti Aug 18, 2025

Uh oh!

agatti Aug 18, 2025

Uh oh!

agatti Aug 18, 2025

Uh oh!

dpgeorge commented Aug 19, 2025

Uh oh!

agatti commented Aug 19, 2025 •

edited

Loading

Uh oh!

MaksimFeng commented Aug 19, 2025

Uh oh!

Uh oh!

Uh oh!

Fix NULL pointer dereference in mp_obj_get_type #17944 #17948

Are you sure you want to change the base?

Fix NULL pointer dereference in mp_obj_get_type #17944 #17948

Uh oh!

Conversation

MaksimFeng commented Aug 17, 2025

Summary

Testing

Uh oh!

codecov bot commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agatti Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpgeorge commented Aug 19, 2025

Uh oh!

agatti commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaksimFeng commented Aug 19, 2025

Uh oh!

Uh oh!

codecov bot commented Aug 17, 2025 •

edited

Loading

github-actions bot commented Aug 17, 2025 •

edited

Loading

agatti Aug 18, 2025 •

edited

Loading

agatti commented Aug 19, 2025 •

edited

Loading