Don't overwrite fstab, even when root mount is missing
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Ubuntu Image |
Fix Released
|
Medium
|
Paul Mars |
Bug Description
Bug reported by Dave Jones (waveform) via github, copy-pasting it here into Launchpad:
As a follow-on to #30, while testing the new ubuntu-image with some proposed livecd-rootfs changes I found another incorrect corner case: if the fstab has no root partition, but contains other entries (e.g. a swap partition, or a boot partition), the entire fstab is clobbered and re-written with one just containing the LABEL=writable root partition.
Admittedly, this should never occur in practice because livecd-rootfs should always produce an fstab with a root mount-point (as indeed it does at the moment), but the logic is still clearly wrong.
The current logic is as follows:
ubuntu-
Lines 186 to 196 in 95689d1
if !strings.
re := regexp.
newContents := re.ReplaceAll(
if !strings.
newContents = []byte(
}
err := ioutilWriteFile
if err != nil {
return fmt.Errorf("Error writing to fstab: %s", err.Error())
}
}
I would suggest this should be changed to:
Always parse the fstab regardless of whether LABEL=writable is present or not (even if it is present, there's no guarantee it's not just in a comment)
Don't assume any existing root mount-point uses a LABEL= scheme (as the current regex does)
Preferably don't use a regex to find the root mount-point. The format is easy enough to parse "properly" by simple whitespace-
Append the root mount-point definition if none are found but leave the rest of the file intact
Eliminate subsequent root mount-point definitions if multiple are found (or would it be preferable to crash'n'burn? It might be...)
Use the correct default mount options and fsck flag
Ensure the file is produced with a newline at the end (to facilitate further script-based processing; the current code outputs a file with no ending NL in the case the file is re-written)
The following awk code implements the above suggestions and could serve as a guide for what needs to be implemented in ubuntu-image (cribbed from my proposed merge for livecd-rootfs). It assumes that root_fs_label and root_fs_options are externally defined (which they are in the script to "cloudimg-rootfs" and "discard,
# Use tab separators on any modified lines
BEGIN { OFS="\t"; count=0; }
# Omit the "UNCONFIGURED" warning if it is still present
/^# UNCONFIGURED FSTAB/ { next; }
# Only modify the first non-comment line where the second field is
# the root and omit multiple root definitions
/^[^#]/ && $2 == "/" {
if (!count) {
$1="LABEL=" root_fs_label;
$6="1";
}
count++;
if (count > 1) next;
}
# Output the (potentially modified) line
{ print; }
# If we reach the end without seeing a root mount line, add one
END {
if (!count) {
print "LABEL=" root_fs_label, "/", "ext4", root_fs_options, "0", "1";
}
}
Changed in ubuntu-image: | |
importance: | Undecided → Medium |
Changed in ubuntu-image: | |
status: | New → In Progress |
assignee: | nobody → Paul Mars (upils) |
Changed in ubuntu-image: | |
status: | In Progress → Fix Released |