Put fluidsynth.lock in its own directory#1528
Merged
derselbst merged 1 commit intoFluidSynth:masterfrom Apr 6, 2025
Merged
Conversation
In some distributions (e.g. Fedora), /run/lock is only writable by root. Create /run/lock/fluidsynth with appropriate permissions. Fixes FluidSynth#1527.
|
derselbst
approved these changes
Apr 6, 2025
Member
derselbst
left a comment
derselbst
left a comment
There was a problem hiding this comment.
Thanks for taking care of this!
I can confirm that the system service file isn't installed by CMake.
For RPM-based distros, this looks fine. I'll include a hint about that in the release notes of the next version, so no need to ping the maintainers, IMO.
DominusExult
added a commit
to DominusExult/fluidsynth-sans-glib
that referenced
this pull request
Apr 7, 2025
* master: (51 commits) Put fluidsynth.lock in its own directory (FluidSynth#1528) Fix XG Drum bank selection (FluidSynth#1514) Stop SDL3 intercepting signals (FluidSynth#1520) Fix MSVC type truncation warnings (FluidSynth#1515) Fix SDL3.dll not included in prebuilt Windows Binaries (FluidSynth#1518) Fix Altitude test case Update testcases for FluidSynth#1508 Apply *_FOR_BUILD environment variables to gentables (FluidSynth#1501) Add testfiles for FluidSynth#1496 and FluidSynth#1505 Remove unused enum Bump to 2.4.4 Fix ordering and dependencies in fluidsynth.service file (FluidSynth#1500) fluidsynth.1: Some remarks and a patch with editorial changes for this man page (FluidSynth#1499) Add support for SDL3 (FluidSynth#1485) Fix interruptions of real-time playback (FluidSynth#1494) Add two more test cases Log SysEX System ON Messages Return error code on server failure Rephrase error messages for clarity Only run one FluidSynth service at a time ...
Contributor
Author
|
For the record, I've used this PR as the basis of the fix for an equivalent issue in bluez. The feedback here has helped significantly over there - thanks! |
github-actions bot
pushed a commit
to tedd-an/bluez-upstream-test
that referenced
this pull request
Apr 18, 2025
Obex is a device-oriented protocol, so only one instance can run per device. But Linux file security is user-oriented, so obexd should be a user service. Tell systemd to only run this service for the first non-system user to log in. Without this patch, errors like the following will occur if you use the "switch account" feature of your desktop environment, then log in with another account: Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00001133-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00001132-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 0000112f-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00001104-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00001106-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00001105-0000-1000-8000-00805f9b34fb which is already registered Mar 26 15:20:38 andrews-2024-laptop bluetoothd[873]: src/profile.c:register_profile() :1.2016 tried to register 00005005-0000-1000-8000-0002ee000001 which is already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered Mar 26 15:20:38 andrews-2024-laptop obexd[1795560]: bluetooth: RequestProfile error: org.bluez.Error.NotPermitted, UUID already registered The above errors indicate the service completely failed to register, so this does not change the user experience beyond removing the above messages. Specifically, the first user who logs in to a multi-user system still retains obex access to devices paired by users who log in later. This is based on a pair of recent changes to the FluidSynth daemon: FluidSynth/fluidsynth#1491 FluidSynth/fluidsynth#1528 This was discussed in the comments for a GitHub advisory available at GHSA-fpgq-25xf-jcwv but comments are not shown in the published version of the advisory. To summarise the useful conversation with Luiz Augusto von Dentz: Obex requires access to the user's home directory, so an attacker can only transfer files between locations the user controls. That may be a problem if the user who runs obexd is different to the user who paired the device, but solving that would involve pairing per-user, which causes other problems. Bluetooth connections can be initiated by peripherals, so switching user could trigger re-pairing and cause the original user to lose access to the device. This may seem reasonable for file access, but for example users would likely object to constantly re-pairing their Bluetooth keyboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In some distributions (e.g. Fedora), /run/lock is only writable by root.
Create /run/lock/fluidsynth with appropriate permissions.
Fixes #1527.
Some issues with rolling this out:
I can't see where the
cmakeinstaller installs the.servicefiles, so haven't added an equivalent to installfluidsynth.tmpfiles. If the installer doesn't do that, this is fine.I've added the relevant line to
contrib/fluidsynth.spec, and the documentation claims%{_tmpfilesdir}is the correct incantation, but I don't have a Fedora system to test on. Should we give the Fedora maintainer a heads-up somehow?I haven't added the relevant line to
contrib/debian.fluidsynth.install, because Debian's version of that file has drifted out of sync. To see their version, go to Debian's package page, download fluidsynth_2.4.4+dfsg-1.debian.tar.xz and take a look atdebian/fluidsynth.install. The relevant line is probably*/fluidsynth.tmpfiles usr/lib/tmpfiles.d/fluidsynth.conf- do you want to resync these, or shall I just send them a bug report explaining they'll need to fix their version?