You are not logged in.
This is unfortunately beyond me.
When allocating the copy's memory
copy->str = g_malloc (copy->allocated_len);
copy->allocated_len is used
copy->allocated_len = string->allocated_len + 0x1000;
which is always bigger than string->len and thus copy->len.
Thanks again for chasing this further and good luck sorting it out.
Offline
copy->allocated_len = string->allocated_len + 0x1000;
And grows by 4kB w/ every "copy" and will eventually lead to math errors - the entire function looks nuts.
A copy means you allocate the data and memcpy it.
Wtf would you change values in a *copy* and wtf would you alter (explicit \0) data in a *COPY*?
Oh, yeah. Forgot.
https://bbs.archlinux.org/viewtopic.php … 3#p2263323
Edit: seeing the upstream bug the problem seems rythmbox' original string creation and the + 0x1000 isn't actually in the upstream code.
Edit #2: because of the questions there, if you're gonna start sanitizing stuff in a copy function, sanitize properly and ensure that allocated_len >= len+1 ?
Last edited by seth (2025-10-05 07:50:52)
Offline
I see.
That makes a lot more sense.
Shino probably added the + 0x1000 to debug the issue.
Offline
I see.
That makes a lot more sense.
Shino probably added the + 0x1000 to debug the issue.
Oh, yes. I accidentally copied my work copy of the function where I tried stuff out.
Edit #2: because of the questions there, if you're gonna start sanitizing stuff in a copy function, sanitize properly and ensure that allocated_len >= len+1 ?
At least to me there is a philosophical problem: The copy function inherently assumes that the string can have arbitrary data in it. Therefore hte need of the len field. However, it terminates it with a zero?!.
Either these GStrings are compatible with a C string (zero terminated, but may not contain zeroes), or not.
From the documentation:
The emphasis of GString is on text, typically UTF-8. Crucially, the “str” member of a GString is guaranteed to have a trailing nul character, and it is therefore always safe to call functions such as strchr() or strdup() on it.
So, in that case rhythmbox is actually doing it wrong by filling the allocated space completely with data.
Last edited by Shino (2025-10-05 12:03:18)
Offline
Shino, I agree that the fix should be in rhythm box and the solution you proposed looks really good.
But I don't understand why the glib guys are reluctant to check len == allocated_len before appending \0 and rely on the documentation.
As seth said: it's their choice to alter the copy to make it more compatible.
Thanks again for the effort you put in to find the best solution.
Last edited by lotan (2025-10-05 18:13:33)
Offline
Shino, I agree that the fix should be in rhythm box and the solution you proposed looks really good.
But I don't understand why the glib guys are reluctant to check len == allocated_len before appending \0 and rely on the documentation.
As seth said: it's their choice to alter the copy to make it more compatible.Thanks again for the effort you put in to find the best solution.
Whatever, I have a working merge request open at rhythmbox: https://gitlab.gnome.org/GNOME/rhythmbo … quests/203
For anyone experiencing htese crashes and not wanting to downgrade GLib:
Build the rhythmbox package from source and add the following lines to your prepare step in PKGBUILD
# Make rhythmbox compatible with latest Glib. Fixes hard memory corruption and crashes.
git cherry-pick -n 7c6441208eb4ed396447d887039696e3147edd93
git cherry-pick -n 0857c46b4f9b36d3119ca9de4dbf1d92752b9d88
Those are the two commits of my pull request.
Build the package normally with makepkg. The crashes should now be fixed.
Offline