Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 401573 - sys-apps/openrc: If /usr is mount and is read-only, remount it into into read-write
Summary: sys-apps/openrc: If /usr is mount and is read-only, remount it into into read...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Hosted Projects
Classification: Unclassified
Component: OpenRC (show other bugs)
Hardware: All Linux
: Normal enhancement (vote)
Assignee: OpenRC Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 399185
  Show dependency tree
 
Reported: 2012-01-30 20:41 UTC by Piotr Karbowski
Modified: 2012-02-09 07:29 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
a patch (openrc-usr-rw.patch,842 bytes, patch)
2012-01-30 20:42 UTC, Piotr Karbowski
Details | Diff
patch v2 (openrc-usr-rw.patch,841 bytes, patch)
2012-01-30 21:15 UTC, Piotr Karbowski
Details | Diff
patch v3 (openrc-remount-rw.patch,1.17 KB, patch)
2012-01-30 22:54 UTC, Piotr Karbowski
Details | Diff
patch v4 (openrc-remount-rw.patch,1.33 KB, patch)
2012-01-30 23:11 UTC, Piotr Karbowski
Details | Diff
patch v5 (openrc-remount-rw.patch,1.47 KB, patch)
2012-01-30 23:21 UTC, Piotr Karbowski
Details | Diff
0001-fstabinfo-add-remount-option.patch (0001-fstabinfo-add-remount-option.patch,2.79 KB, text/plain)
2012-01-31 18:06 UTC, William Hubbs
Details
make root initscript remount all mounted filesystems. (remount.patch,1.60 KB, patch)
2012-02-03 20:36 UTC, Piotr Karbowski
Details | Diff
0001-Remount-already-mounted-filesystems.patch (0001-Remount-already-mounted-filesystems.patch,1.68 KB, text/plain)
2012-02-06 16:35 UTC, William Hubbs
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 20:41:47 UTC
After talking with William about mounting /usr in initramfs level, we agreed that /usr should be remounted into read-write in localmount.

Reproducible: Always
Comment 1 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 20:42:20 UTC
Created attachment 300467 [details, diff]
a patch
Comment 2 William Hubbs gentoo-dev 2012-01-30 21:08:51 UTC
I have one concern about this patch:

mountpoint shouldn't be used since it is lnux/sysvinit specific. Please
rework the patch so it doesn't use this.

Thanks,

William
Comment 3 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 21:15:32 UTC
Created attachment 300471 [details, diff]
patch v2

Okey. reworked.
Comment 4 SpanKY gentoo-dev 2012-01-30 21:20:16 UTC
Comment on attachment 300471 [details, diff]
patch v2

unless i missed something, this will always mount /usr rw.  that is wrong.  if the user has set /usr in their /etc/fstab to be ro, then openrc should not be overriding that.
Comment 5 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 21:27:02 UTC
You are right.

Do you have idea how we can handle it?

The only solution what I see is loop over /etc/fstab, check for /usr mountpoint (if exist) and then check in fashion:

for flag in ${fsflags//,/ }; do if [ "${flag}" = 'ro' ]; then no_usr_remount=true; fi; done
Comment 6 SpanKY gentoo-dev 2012-01-30 22:03:27 UTC
err, no ... we have `fstabinfo` already for querying fstab setups.  and it has a --mount flag.  probably easy to add a remount flag so that fstabinfo would automatically remount it with the fstab settings (if it doesn't already).
Comment 7 William Hubbs gentoo-dev 2012-01-30 22:06:48 UTC
Check out the fstabinfo helper located in $LIBEXECDIR/bin/fstabinfo.
You may be able to use that to look up whether /usr is set up to be ro
in fstab.
Comment 8 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 22:24:43 UTC
This needs to be more general.
For all mountpoints considered in localmount, we should do this at boot, not just /usr. This can then also partially replace the 'root' init script.
Comment 9 William Hubbs gentoo-dev 2012-01-30 22:29:52 UTC
WRT the --remount flag possibility:
Can Can this also be done in the *bsd world? I know Linux has a remount
mount option, but I don't know how the BSD's do this.
Comment 10 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 22:34:05 UTC
For the --remount option, to make it suit this usecase it would need to check for the ro option as well, if there is no such option and mountpoint is mounted with ro, it should append 'rw' to options.

For now I will copy the 'case' statement from root init script.

@Robin: Sounds good. I will roll a new patch in a few.
Comment 11 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 22:54:02 UTC
Created attachment 300485 [details, diff]
patch v3

Okey, now script will do for-loop on each element from fstabinfo starting with /, it will check if the the path is contain mounted fs and if it is read-only, then it will check for ro flag in fstab for this mountpoint, if there is none, it will run remount,rw on it.
Comment 12 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 23:11:24 UTC
Created attachment 300487 [details, diff]
patch v4

Update, case statement for the RC_UNAME, *BSD does not have -o remount, they use -u to update already-mounted mountpoint.
Comment 13 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-01-30 23:21:46 UTC
Created attachment 300489 [details, diff]
patch v5

if mountpoint is /, append -n (no mtab update).

Sorry about all teh updates.
Comment 14 SpanKY gentoo-dev 2012-01-30 23:53:18 UTC
Comment on attachment 300489 [details, diff]
patch v5

there's no need to parse the fstabinfo like this.  just always call `mount -o remount` if it's found in the fstab.  mount is smart enough to load the rw opts from /etc/fstab.

that said, i still think this should just be part of fstabinfo as a remount flag.
Comment 15 William Hubbs gentoo-dev 2012-01-31 18:06:54 UTC
Created attachment 300553 [details]
0001-fstabinfo-add-remount-option.patch

Please review this patch which adds a --remount/-R option to fstabinfo.
Comment 16 William Hubbs gentoo-dev 2012-01-31 20:48:34 UTC
(In reply to comment #8)
> This needs to be more general.
> For all mountpoints considered in localmount, we should do this at boot, not
> just /usr. This can then also partially replace the 'root' init script.

My patch in the previous comment gives us the --remount option for fstabinfo. Once that is in the tree, the next step would be to extend the root init script to remount any other filesystems that are mounted read-only based on the contents of fstab for those filesystems.
Comment 17 William Hubbs gentoo-dev 2012-01-31 23:07:49 UTC
Commit 0fcc625 contains a slightly modified version of this patch; the
modifications were suggested by vapier.

The next stage of this fix is to add code to the root init.d script so
that after it remounts root it attempts to remount any filesystems that
are mounted read-only but should not be based on their fstab entries.
Comment 18 SpanKY gentoo-dev 2012-02-01 16:15:49 UTC
i think the logic is off there.  the init.d script should remount any fs that was previously mounted and is listed in /etc/fstab.  there's no need to parse the options exactly and compare them ... i think that's slower than simply remounting them all and letting mount/kernel sort it out.
Comment 19 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-02-03 20:36:46 UTC
Created attachment 300879 [details, diff]
make root initscript remount all mounted filesystems.
Comment 20 William Hubbs gentoo-dev 2012-02-06 16:35:22 UTC
Created attachment 301041 [details]
0001-Remount-already-mounted-filesystems.patch

Piotr and all, this is a slightly modified version of the patch.

In this version, I changed the code to use the checkpath helper and
reworked the "if eend ...; then" line to be more readable.

What do you think of this version?

Thanks,

William
Comment 21 Piotr Karbowski archtester Gentoo Infrastructure gentoo-dev Security 2012-02-06 17:06:10 UTC
Looks good for me.
Comment 22 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2012-02-09 07:29:21 UTC
Ok, it passes all of my dogfood testing with the /usr mounting in genkernel, so I've committed it to the tree as commit 497ff7e.

Thanks for the submission and work!