Bugzilla – Bug 16506
Squeezebox Server fails to scan directories it does not own
Last modified: 2014-06-05 08:46:40 UTC
On Ubuntu, it seems that squeezebox server will fail to scan music directories that unless its user (squeezeboxserver): a) owns the directories, or b) is a member of the owning group, or c) the directories are world readable/executable In the case that the squeezebox server user is a member of a secondary group that has access to the directories (e.g., via POSIX ACLs) but none of the above conditions are met, SBS fails to scan the directories. I have tested this by assigning the squeezeboxserver user a shell and switching to that user via su. As that user, I am able to browse around and access the music files. However SBS still fails to access them. I have described the situation in the following forum post, including evidence copied from terminal sessions. http://forums.slimdevices.com/showthread.php?t=81742 It is almost as if SBS is checking ownership of the files and failing to open them, rather than allowing the operating system to arbitrate file access and then handling resulting errors.
That's a function of the operating system and is working as expected.
I may not have explained the problem well. Please let me know if clarification would help. Why is it that the even when the 'squeezeboxserver' user can access the directories and files, the SBS application running as that user is unable to. This isn't expected behavior.
I think I see what's causing this behavior. As the function fileFilter() first recurses over all the directories and files, it stat()s each one in turn. Then it checks the result from stat to make sure the file is a directory, symlink or regular file, and is readable: Misc.pm: fileFilter() # We only want files, directories and symlinks Bug #441 # Otherwise we'll try and read them, and bad things will happen. # symlink must come first so an lstat() is done. if ( !$hasStat ) { lstat($fullpath); } return 0 unless (-l _ || -d _ || -f _); # Make sure we can read the file. return 0 if !-r _; Probably, perl's -r operation on the stat() result performs an evaluation of the permission bits and ownership, and checks those against the current effective UID and GID. Unfortunately this check is unreliable in some cases. One is the case of this bug where the file has secondary group or user accesses defined in file ACLs (Mac OS X, Windows, and Linux all support granular file ACLs). Another case is where the system is implements a form of Mandatory Access Control restricting access more than what is allowed by the user & group ownership and permission bits. Examples include Suse & Ubuntu running AppArmor as well as Fedora, RHEL and CentOS running SELinux. A more robust option, if possible, would be to attempt to open the file for reading (or descend into the directory) and check for success.
I've made a one-line change that, so far, has addressed the anomalous behavior with no apparent side effects. I'd like to continue testing for a bit longer, but then I'll post a patch if that's acceptable. Thanks
I can help testing on an AFS and NFSv3/NFSv4 installation. I'm suffering the same strange behaviour with SBS accessing files on AFS and NFS. Sidenote for AFS: Its ACLs don't really affect UNIX Mode bits, especially for group and other. That means as a workaround I could do a "chmod a+r(x)" without opening the files for the world because for AFS the ACL entries count and the ACL entries didn't get changed by the chmod -- for NFSv4 on Solaris ZFS they did get adapted, hence I used the poster's workaround by group-owning the files/directories by a group the SBS belongs to. HTH -- SEBi
I've patched my copy of SBS. Although it seems to be working fine nwo, I haven't had a chance to test it as thoroughly as I'd like. It's a relatively easy fix though. Basically, wherever SBS checks if the file is readable using perl's -r, the fix is to simply eliminate that check. This isn't a robust check, as it doesn't allow the operating system to arbitrate file access, and therefore doesn't account for things like file ACLs, or mandatory access control policies (e.g., AppArmor, SELinux, etc). Further, even if the '-r' check passes when the music is being indexed, the file permissions may change before SBS actually attempts to open the file for playback (hours or days later). So SBS still has to handle the case the file being absent or unreadable. Rather than having SBS arbitrate file access, SBS should simply attempt to open the file, and check for success or failure, then handle failure gracefully. I've found if I simply comment out the -r check, SBS does just that. The problem is complicated somewhat by the fact that these naive -r checks are found throughout the code, including plug-ins, so it's a bit more than a simple one-line change.
Created attachment 7318 [details] Patch: Honor read access given by ACL (using "access" instead of stat) Perl filetest "-r" only chakc for the flags/bits returned by stat(). If the read access is given via ACLs (setfacl ...), this does not work. This patch makes perl honor the ACLs, too.
There shouldn't be a need for checking for access to the file. If we simply take out the '-r' check, the program will attempt to use the file and it fails gracefully if it can't. I'm not familiar with "access", but even if it's portable and can address all cases of ACLs of mandatory access controls, it's racy to check for access before accessing a file. The permissions on the file can change between the "-r" check and when the program actually attempts to use the file. The file could even be deleted. In any case, the program still must handle access failures gracefully. In my patched version, I simply comment out the "-r" check and SBS works fine. (In reply to comment #7) > Created an attachment (id=7318) [details] > Patch: Honor read access given by ACL (using "access" instead of stat) > > Perl filetest "-r" only chakc for the flags/bits returned by stat(). If the > read access is given via ACLs (setfacl ...), this does not work. > > This patch makes perl honor the ACLs, too.
I can confirm this bug. I am running Slackware64 13.1, and the user running the Logitech Media Server on my system has read access to my music folder via ACL, and can access the folder in the terminal without problems. Accessing the folder via Logitech Media Server does not work until you patch the Misc.pm file, as suggested by junkmael99@gmail.com. Thanks. Great work :-)
> Accessing the folder via Logitech Media Server does not work until you patch > the Misc.pm file, as suggested by junkmael99@gmail.com. This saved my day, many thanks to junkmael99@gmail.com.
(In reply to comment #8) > There shouldn't be a need for checking for access to the file. If we simply > take out the '-r' check, the program will attempt to use the file and it > fails gracefully if it can't. As your write, this check may not be required at all. But this patch is most closely to the intented funtion: check whethter the uer can actually read the file. If somebody wants to remove this check complety, this is a different topic.
I just issued pull #14 and pull #15.
Ok, let's see what we can do here. Trying to understand the suggested solutions: - remove the -r check and let things fail: most likely there will be a performance hit, as checking the permission bits would be faster - but incomplete - use filetest: adding overhead, too - afaict this is only an issue on unixy systems, but not Windows IMHO we're trying to fix a corner case which most people don't care about, because they don't encounter it. Therefore I'm reluctant to take a performance hit for everybody to fix an issue only a very small group of users is seeing. Could some of you please run some timing tests to see how the time used for a full scan changes with and without "use filetest"? I don't care about the race condition, as there will always be an even longer period of time between the scan and the actual use of a file (listening to it), during which it's much more likely to see a permissions change than during the scan. I hate to suggest this... but shall we make this a user setting, turned off by default?
A quick & dirty test (recursive scan of my music directory with a -r test on any file found) shows that there's obviously some overhead when using the ACL-compatible directive: seb@lms:~$ ./test.pl /media/musique/flac PASS #1, MODE=STD, FILES=22762, DURATION=5 PASS #2, MODE=ACL, FILES=22762, DURATION=16 PASS #3, MODE=STD, FILES=22762, DURATION=7 PASS #4, MODE=ACL, FILES=22762, DURATION=11 PASS #5, MODE=STD, FILES=22762, DURATION=2 PASS #6, MODE=ACL, FILES=22762, DURATION=9 However, it's not that significant in the grand scheme of things, IMO: - Discovering files/directories: /media/musique/flac (22354 of 22354) Complete 00:00:35 - Scanning new music files: /media/musique/flac (20016 of 20016) Complete 00:08:26 - Discovering files/directories: /media/musique/mp3 (70 of 70) Complete 00:00:00 - Scanning new music files: /media/musique/mp3 (51 of 51) Complete 00:00:05 - Discovering playlists: /media/musique/playlists (4 of 4) Complete 00:00:01 - Scanning new playlists: /media/musique/playlists (3 of 3) Complete 00:00:04 - MusicIP Import (20191 of 20191) Complete 00:04:52 - Pre-caching Artwork (1348 of 1348) Complete 00:02:01 The server has finished scanning your media library. Total Time: 00:16:04 (mardi 8 avril 2014 / 19:42) I need to set a new VM up to make further tests, because removing the patch from my current setup would break it without a doubt, but I will try to provide solid numbers. And yes, it's a corner case for people using POSIX ACLs on Linux (and possibly other *NIX systems). But POSIX ACLs are a very nice option to have, especially when using NFSv4, which I do, and that's why some people will use them! ;) Can only speak for myself, but I'd be perfectly happy with a --use-filetest-access option that I could use in my startup script. It really doesn't need to be enabled by default, IMO, but patching any released version again and again to fix this issue is a bit tedious...
So... My tests were done on a Debian Wheezy pVM running on my HP Proliant microserver. The filesystem holding the copy of my music library was dismounted and remounted before each test run to prevent any FS caching effect, and the full LMS archive was freshly extracted and reconfigured. Like any time I do, I applied the patch attached to this bug report minus the $scannerlog->error() call (that really is some debug info, and a true performance killer). Vanilla LMS 7.8.0 ======================================================================== Discovering files/directories: /media/musique/flac (22354 of 22354) Complete 00:00:09 Scanning new music files: /media/musique/flac (20016 of 20016) Complete 00:08:10 Discovering playlists: /media/musique/playlists (4 of 4) Complete 00:00:00 Scanning new playlists: /media/musique/playlists (3 of 3) Complete 00:00:03 Pre-caching Artwork (1344 of 1344) Complete 00:01:18 The server has finished scanning your media library. Total Time: 00:09:40 (Wednesday 9 April 2014 / 00:29) Patched LMS 7.8.0 ======================================================================== Discovering files/directories: /media/musique/flac (22354 of 22354) Complete 00:00:10 Scanning new music files: /media/musique/flac (20016 of 20016) Complete 00:08:05 Discovering playlists: /media/musique/playlists (4 of 4) Complete 00:00:00 Scanning new playlists: /media/musique/playlists (3 of 3) Complete 00:00:04 Pre-caching Artwork (1344 of 1344) Complete 00:01:16 The server has finished scanning your media library. Total Time: 00:09:35 (Wednesday 9 April 2014 / 01:05) Looks like the patch had absolutely no impact in this setting. My production LMS server accesses the music library through a NFS share whereas this test VM had direct I/O access to an EXT4 partition. May explain why overhead was visible there but ain't here. Whatever, looks like the impact should be barely noticeable.
Thanks for those tests! What's the hardware specs of your server? Anyone else tested the impact of the suggested fix? I'm going to run my own tests on some more systems to get a feel for it.
(In reply to comment #16) > Thanks for those tests! What's the hardware specs of your server? You're welcome! It's a "HP ProLiant N54L", coming with an AMD Turion II Neo processor, equipped with 16GB RAM (I run a bunch of Linux VMs), a 64GB SSD holding the host and guests system partitions, and three 2TB HDDs to hold all the regular data, with some RAID-1 mirrors set up at a logical volume level to protect the most sensitive data (including my beloved FLAC collection, obviously).
git 5d59e9dd3848481004db841ad0f74a1f0cf53e1b - merged to 7.9. Thanks htgoebel!
Thanks!
*** Bug 18002 has been marked as a duplicate of this bug. ***
I'm sorry, I'll revert that change again. It's just not compatible enough, causes crashes on too many systems we've seen. I'll try to come up with an alternative approach. Thanks for your understanding.
I've moved the ACL check to an optional plugin. It will be available with tomorrow's build. Go to Settings/Plugins to enable the ACL check plugin. Please let me know how this works for you.