/etc/init.d/{consolefont,keymaps,termencoding} have the following checks: -w "$RC_LIBEXECDIR" Besides the question of why that code exists (certain Initramfs?), that check doesn't discover a read-only filesystem if the shell is dash, for example. -w file True if file exists and is writable. True indicates only that the write flag is on. The file is not writable on a read-only file system even if this test indicates true. Moreover, since the user running that command is root, directory's permissions aren't even considered - the result is always positive.
Created attachment 299087 [details] dir_writable.patch This patch should take care of making sure that the filesystem is actually writable. I will apply this if this is the way we want to go. Can someone else provide input to the bug though wrt why we need to test this since all of these scripts run after localmount?
(In reply to comment #1) > This patch should take care of making sure that the filesystem is > actually writable. Why not just: if mkdir "$RC_LIBEXECDIR"/console 2>/dev/null; then > Can someone else provide input to the bug though wrt why we need to > test this since all of these scripts run after localmount? /lib can be on read-only fs -- see the last two paragraphs at http://freedesktop.org/wiki/Software/systemd/separate-usr-is-broken, for instance. What I don't understand is why that code exists at all. Is it for Gentoo's Initramfs? Is that standard?
(In reply to comment #2) > (In reply to comment #1) > > This patch should take care of making sure that the filesystem is > > actually writable. > > Why not just: > > if mkdir "$RC_LIBEXECDIR"/console 2>/dev/null; then This will fail if $RC_LIBEXECDIR/console already exists and not really check to see if the directory is writable. > What I don't understand is why that code exists at all. Is it for Gentoo's > Initramfs? Is that standard? I'm working on getting an answer for this, but, if we keep the code, I think the patch I attached is going to be the best way to fix the test based on what I said above.
(In reply to comment #3) > This will fail if $RC_LIBEXECDIR/console already exists and not really check > to see if the directory is writable. if /usr/bin/test -w "$RC_LIBEXECDIR"; then
(In reply to comment #4) > (In reply to comment #3) > > This will fail if $RC_LIBEXECDIR/console already exists and not really check > > to see if the directory is writable. > > if /usr/bin/test -w "$RC_LIBEXECDIR"; then This is basically the same test we already have, unless it also checks to see if the filesystem is mounted read-only. The man page for test just says that -w makes sure the file exists and write permission is granted, so I'm not sure whether it checks the mount status or not.
Hi, I know that it probably seems that I'm just throwing suggestions, but I actually tried a lot of stuff before finding that solution. How do you like: [ "$(blockdev --getro /dev/block/"$(mountpoint -d "$RC_LIBEXECDIR" 2>/dev/null)" 2>/dev/null)" = 0 ] Anyway, I didn't look in source code of coreutils, but I did look at what bash does, and it essentially calls access(2). I assume that coreutils does the same. From manpage of access(): R_OK, W_OK, and X_OK test whether the file exists and _grants_ read, write, and execute permissions, respectively. EROFS Write permission was requested for a file on a read-only file system. Compare it with test(1): -w FILE FILE exists and write permission is _granted_ Now, you can test ro filesystems yourself easily by doing mount -B, followed by mount -o remount,ro.
(In reply to comment #6) > Hi, I know that it probably seems that I'm just throwing suggestions, but I > actually tried a lot of stuff before finding that solution. How do you like: > > [ "$(blockdev --getro /dev/block/"$(mountpoint -d "$RC_LIBEXECDIR" > 2>/dev/null)" 2>/dev/null)" = 0 ] Openrc runs on linux, FreeBSD and NetBSD, so we need to be sure the test we use works for the *BSDs as well. I'm not sure, but I think this is linux specific. > Anyway, I didn't look in source code of coreutils, but I did look at what bash > does, and it essentially calls access(2). I assume that coreutils does the > same. > > From manpage of access(): > > R_OK, W_OK, and X_OK test whether the file exists and _grants_ read, > write, and execute permissions, respectively. > > EROFS Write permission was requested for a file on a read-only file > system. > > Compare it with test(1): > > -w FILE FILE exists and write permission is _granted_ I'm not following your comparison. What do you want me to see here? > Now, you can test ro filesystems yourself easily by doing mount -B, followed by > mount -o remount,ro. I'm not following how using a bind mount would test to see if something is mounted read only.
Ok, I will keep it simple. 1. Extract coreutils-8.7.tar.xz 2. Look in src/test.c 3. Look in lib/euidaccess.c 4. Read access(2) man page 5. Mount a read-only filesystem 6. Execute /usr/bin/test -w If the above is hard, please escalate the bug to someone who knows to program. Thanks.
I talked this over with the team, and we decided that the best fix is my patch in comment #1. This has been applied in commit 6e2fbf6. Thanks for the report.
Your patch is not a fix, but a hack due to incompetence. Please escalate the issue to someone who knows about C and POSIX. Additionally, the question of necessity of that code has not been resolved.
I did discover why it is there. See /lib/rc/sh/init-early.sh. That is where these settings are loaded. It is a way on linux for us to load the settings as early as possible. The other important thing here is that "[" and "test" are identical, so they perform the same test. I have spoken, as I said, with several team members, and we agreed that this is the fix.
Ok, good to know about init-early.sh. I regret that you can't see why /usr/bin/test -w is a better solution that doesn't cause unnecessary writes to the filesystem, though. There is no excuse for producing unnecessarily inferior code.
Maxim: Are you ok with http://git.overlays.gentoo.org/gitweb/?p=proj/openrc.git;a=commitdiff;h=7ea5c614d9c9e36c55f1da3d7fb894e83bbb56f3 ? Thus using "checkpath -W /path/to/foo" instead?
Sure, sounds good, especially since there is no need to access /usr, - although it seems that checkpath has somewhat different purpose, but I guess you know better. Also, is_writable() is not defined in that commit, so I assume that it calls access().
(In reply to comment #14) > Sure, sounds good, especially since there is no need to access /usr, - although > it seems that checkpath has somewhat different purpose, but I guess you know > better. Also, is_writable() is not defined in that commit, so I assume that it > calls access(). http://git.overlays.gentoo.org/gitweb/?p=proj/openrc.git;a=commitdiff;h=44019f6542885fd684c5113c7a5c06308a51102a That was the right commit, sorry.
Yeah, access(..., W_OK) - as simple as it gets.
