| Message ID | 20250327084128.3315736-1-quic_shuaz@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] dbus: Fix add invalid memory during interface removal | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=947676 ---Test result--- Test Summary: CheckPatch PENDING 0.21 seconds GitLint PENDING 0.21 seconds BuildEll PASS 20.58 seconds BluezMake PASS 1463.81 seconds MakeCheck PASS 13.21 seconds MakeDistcheck PASS 156.36 seconds CheckValgrind PASS 211.73 seconds CheckSmatch PASS 281.97 seconds bluezmakeextell PASS 97.21 seconds IncrementalBuild PENDING 0.28 seconds ScanBuild PASS 859.96 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
Hi Shuai, On Thu, Mar 27, 2025 at 4:41 AM Shuai Zhang <quic_shuaz@quicinc.com> wrote: > > test setp > register_service <uuid> > register_application <uuid> > unregister_service <uuid> > unregister_application > register_service <uuid> > register_application <uuid> > core dump Ok, what exactly are you talking about above, you are not referring to D-Bus api it seems, are these bluetoothctl commands? > invalidate_parent_data is called to add the service to the application's > glist when unregister_service. However, this service has already been > added to the glist of root object in register_service. This results in > services existing in both queues,but only the services in the > application's glist are freed upon removal. A null address is stored > in root object's glist, a crash dump will occur when get_object is called. > > Add a check for the parent pointer to avoid adding the service again. > > 0 0x0000007ff7df6058 in dbus_message_iter_append_basic () > from /usr/lib/libdbus-1.so.3 > 1 0x00000055555a3780 in append_object (data=0x31306666, > user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1117 > 2 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 > 3 0x00000055555a37ac in append_object (data=0x5555642cf0, > user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1122 > 4 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 > 5 0x00000055555a3630 in get_objects (connection=<optimized out>, > message=<optimized out>, user_data=0x555563b390) > at /usr/src/debug/bluez5/5.72/gdbus/object.c:1154 > 6 0x00000055555a51d0 in process_message ( > connection=connection@entry=0x5555639310, > message=message@entry=0x5555649ac0, > method=method@entry=0x55555facf8 <manager_methods>, > iface_user_data=<optimized out>) > at /usr/src/debug/bluez5/5.72/gdbus/object.c:246 > 7 0x00000055555a575c in generic_message (connection=0x5555639310, > message=0x5555649ac0, user_data=<optimized out>) > > Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com> > --- > gdbus/object.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gdbus/object.c b/gdbus/object.c > index 7b0476f1a..d87a81160 100644 > --- a/gdbus/object.c > +++ b/gdbus/object.c > @@ -809,7 +809,8 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn, > > if (child == NULL || g_slist_find(data->objects, child) != NULL) > goto done; > - > + if(g_slist_find(parent->objects, child) != NULL) > + goto done; Use empty lines after if statements, and keep the one you are removing. > data->objects = g_slist_prepend(data->objects, child); > child->parent = data; > > -- > 2.34.1 > >
Hi Shuai, On Thu, Mar 27, 2025 at 10:58 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Shuai, > > On Thu, Mar 27, 2025 at 4:41 AM Shuai Zhang <quic_shuaz@quicinc.com> wrote: > > > > test setp > > register_service <uuid> > > register_application <uuid> > > unregister_service <uuid> > > unregister_application > > register_service <uuid> > > register_application <uuid> > > core dump > > Ok, what exactly are you talking about above, you are not referring to > D-Bus api it seems, are these bluetoothctl commands? Tried this with bluetoothctl, doesn't seem to trigger any errors: https://gist.github.com/Vudentz/7b20ff8b59e3122606a2239e1b63aa8a > > > invalidate_parent_data is called to add the service to the application's > > glist when unregister_service. However, this service has already been > > added to the glist of root object in register_service. This results in > > services existing in both queues,but only the services in the > > application's glist are freed upon removal. A null address is stored > > in root object's glist, a crash dump will occur when get_object is called. > > > > Add a check for the parent pointer to avoid adding the service again. > > > > 0 0x0000007ff7df6058 in dbus_message_iter_append_basic () > > from /usr/lib/libdbus-1.so.3 > > 1 0x00000055555a3780 in append_object (data=0x31306666, > > user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1117 > > 2 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 > > 3 0x00000055555a37ac in append_object (data=0x5555642cf0, > > user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1122 > > 4 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 > > 5 0x00000055555a3630 in get_objects (connection=<optimized out>, > > message=<optimized out>, user_data=0x555563b390) > > at /usr/src/debug/bluez5/5.72/gdbus/object.c:1154 > > 6 0x00000055555a51d0 in process_message ( > > connection=connection@entry=0x5555639310, > > message=message@entry=0x5555649ac0, > > method=method@entry=0x55555facf8 <manager_methods>, > > iface_user_data=<optimized out>) > > at /usr/src/debug/bluez5/5.72/gdbus/object.c:246 > > 7 0x00000055555a575c in generic_message (connection=0x5555639310, > > message=0x5555649ac0, user_data=<optimized out>) > > > > Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com> > > --- > > gdbus/object.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/gdbus/object.c b/gdbus/object.c > > index 7b0476f1a..d87a81160 100644 > > --- a/gdbus/object.c > > +++ b/gdbus/object.c > > @@ -809,7 +809,8 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn, > > > > if (child == NULL || g_slist_find(data->objects, child) != NULL) > > goto done; > > - > > + if(g_slist_find(parent->objects, child) != NULL) > > + goto done; > > Use empty lines after if statements, and keep the one you are removing. > > > > data->objects = g_slist_prepend(data->objects, child); > > child->parent = data; > > > > -- > > 2.34.1 > > > > > > > -- > Luiz Augusto von Dentz
Hi Luiz On 3/27/2025 10:58 PM, Luiz Augusto von Dentz wrote: > Hi Shuai, > > On Thu, Mar 27, 2025 at 4:41 AM Shuai Zhang <quic_shuaz@quicinc.com> wrote: >> >> test setp >> register_service <uuid> >> register_application <uuid> >> unregister_service <uuid> >> unregister_application >> register_service <uuid> >> register_application <uuid> >> core dump > > Ok, what exactly are you talking about above, you are not referring to > D-Bus api it seems, are these bluetoothctl commands? > Yes,this is bluetoothctl command. I will add explanation in new patch. >> invalidate_parent_data is called to add the service to the application's >> glist when unregister_service. However, this service has already been >> added to the glist of root object in register_service. This results in >> services existing in both queues,but only the services in the >> application's glist are freed upon removal. A null address is stored >> in root object's glist, a crash dump will occur when get_object is called. >> >> Add a check for the parent pointer to avoid adding the service again. >> >> 0 0x0000007ff7df6058 in dbus_message_iter_append_basic () >> from /usr/lib/libdbus-1.so.3 >> 1 0x00000055555a3780 in append_object (data=0x31306666, >> user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1117 >> 2 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 >> 3 0x00000055555a37ac in append_object (data=0x5555642cf0, >> user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1122 >> 4 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 >> 5 0x00000055555a3630 in get_objects (connection=<optimized out>, >> message=<optimized out>, user_data=0x555563b390) >> at /usr/src/debug/bluez5/5.72/gdbus/object.c:1154 >> 6 0x00000055555a51d0 in process_message ( >> connection=connection@entry=0x5555639310, >> message=message@entry=0x5555649ac0, >> method=method@entry=0x55555facf8 <manager_methods>, >> iface_user_data=<optimized out>) >> at /usr/src/debug/bluez5/5.72/gdbus/object.c:246 >> 7 0x00000055555a575c in generic_message (connection=0x5555639310, >> message=0x5555649ac0, user_data=<optimized out>) >> >> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com> >> --- >> gdbus/object.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/gdbus/object.c b/gdbus/object.c >> index 7b0476f1a..d87a81160 100644 >> --- a/gdbus/object.c >> +++ b/gdbus/object.c >> @@ -809,7 +809,8 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn, >> >> if (child == NULL || g_slist_find(data->objects, child) != NULL) >> goto done; >> - >> + if(g_slist_find(parent->objects, child) != NULL) >> + goto done; > > Use empty lines after if statements, and keep the one you are removing. > I will modify it, thanks. > >> data->objects = g_slist_prepend(data->objects, child); >> child->parent = data; >> >> -- >> 2.34.1 >> >> > >
Hi Luiz On 3/27/2025 11:10 PM, Luiz Augusto von Dentz wrote: > Hi Shuai, > > On Thu, Mar 27, 2025 at 10:58 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: >> >> Hi Shuai, >> >> On Thu, Mar 27, 2025 at 4:41 AM Shuai Zhang <quic_shuaz@quicinc.com> wrote: >>> >>> test setp >>> register_service <uuid> >>> register_application <uuid> >>> unregister_service <uuid> >>> unregister_application >>> register_service <uuid> >>> register_application <uuid> >>> core dump >> >> Ok, what exactly are you talking about above, you are not referring to >> D-Bus api it seems, are these bluetoothctl commands? > > Tried this with bluetoothctl, doesn't seem to trigger any errors: > > https://gist.github.com/Vudentz/7b20ff8b59e3122606a2239e1b63aa8a > This error is a probabilistic issue, and the frequency of reproduction is quite high. I have left the reproduction commit in your GitHub glist. >> >>> invalidate_parent_data is called to add the service to the application's >>> glist when unregister_service. However, this service has already been >>> added to the glist of root object in register_service. This results in >>> services existing in both queues,but only the services in the >>> application's glist are freed upon removal. A null address is stored >>> in root object's glist, a crash dump will occur when get_object is called. >>> >>> Add a check for the parent pointer to avoid adding the service again. >>> >>> 0 0x0000007ff7df6058 in dbus_message_iter_append_basic () >>> from /usr/lib/libdbus-1.so.3 >>> 1 0x00000055555a3780 in append_object (data=0x31306666, >>> user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1117 >>> 2 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 >>> 3 0x00000055555a37ac in append_object (data=0x5555642cf0, >>> user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1122 >>> 4 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 >>> 5 0x00000055555a3630 in get_objects (connection=<optimized out>, >>> message=<optimized out>, user_data=0x555563b390) >>> at /usr/src/debug/bluez5/5.72/gdbus/object.c:1154 >>> 6 0x00000055555a51d0 in process_message ( >>> connection=connection@entry=0x5555639310, >>> message=message@entry=0x5555649ac0, >>> method=method@entry=0x55555facf8 <manager_methods>, >>> iface_user_data=<optimized out>) >>> at /usr/src/debug/bluez5/5.72/gdbus/object.c:246 >>> 7 0x00000055555a575c in generic_message (connection=0x5555639310, >>> message=0x5555649ac0, user_data=<optimized out>) >>> >>> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com> >>> --- >>> gdbus/object.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/gdbus/object.c b/gdbus/object.c >>> index 7b0476f1a..d87a81160 100644 >>> --- a/gdbus/object.c >>> +++ b/gdbus/object.c >>> @@ -809,7 +809,8 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn, >>> >>> if (child == NULL || g_slist_find(data->objects, child) != NULL) >>> goto done; >>> - >>> + if(g_slist_find(parent->objects, child) != NULL) >>> + goto done; >> >> Use empty lines after if statements, and keep the one you are removing. >> >> >>> data->objects = g_slist_prepend(data->objects, child); >>> child->parent = data; >>> >>> -- >>> 2.34.1 >>> >>> >> >> >> -- >> Luiz Augusto von Dentz > > >
diff --git a/gdbus/object.c b/gdbus/object.c index 7b0476f1a..d87a81160 100644 --- a/gdbus/object.c +++ b/gdbus/object.c @@ -809,7 +809,8 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn, if (child == NULL || g_slist_find(data->objects, child) != NULL) goto done; - + if(g_slist_find(parent->objects, child) != NULL) + goto done; data->objects = g_slist_prepend(data->objects, child); child->parent = data;
test setp register_service <uuid> register_application <uuid> unregister_service <uuid> unregister_application register_service <uuid> register_application <uuid> core dump invalidate_parent_data is called to add the service to the application's glist when unregister_service. However, this service has already been added to the glist of root object in register_service. This results in services existing in both queues,but only the services in the application's glist are freed upon removal. A null address is stored in root object's glist, a crash dump will occur when get_object is called. Add a check for the parent pointer to avoid adding the service again. 0 0x0000007ff7df6058 in dbus_message_iter_append_basic () from /usr/lib/libdbus-1.so.3 1 0x00000055555a3780 in append_object (data=0x31306666, user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1117 2 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 3 0x00000055555a37ac in append_object (data=0x5555642cf0, user_data=0x7ffffff760) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1122 4 0x0000007ff7ece0cc in g_slist_foreach () from /usr/lib/libglib-2.0.so.0 5 0x00000055555a3630 in get_objects (connection=<optimized out>, message=<optimized out>, user_data=0x555563b390) at /usr/src/debug/bluez5/5.72/gdbus/object.c:1154 6 0x00000055555a51d0 in process_message ( connection=connection@entry=0x5555639310, message=message@entry=0x5555649ac0, method=method@entry=0x55555facf8 <manager_methods>, iface_user_data=<optimized out>) at /usr/src/debug/bluez5/5.72/gdbus/object.c:246 7 0x00000055555a575c in generic_message (connection=0x5555639310, message=0x5555649ac0, user_data=<optimized out>) Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com> --- gdbus/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)