Skip to content

df: error on over-mounted device #7116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 1 commit into from
Jan 12, 2025
Merged

df: error on over-mounted device #7116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
df: error on over-mounted device 
Make `df` error when two devices are mounted at the same directory and
the earlier one is given as a positional argument to `df`. This is
called "over-mounting". After this commit, an error message is printed
like this:

    df: cannot access '/dev/loop1': over-mounted by another device

Fixes #3970

Co-authored-by: Jeffrey Finkelstein <[email protected]>
  • Loading branch information
@ctsk @jfinkels
ctsk and jfinkels committed Jan 11, 2025
commit 02e49a40c03f2fb08e0d778cdb9b157443b90709
55 changes: 39 additions & 16 deletions src/uu/df/src/df.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use std::path::Path;
use crate::blocks::{read_block_size, BlockSize};
use crate::columns::{Column, ColumnError};
use crate::filesystem::Filesystem;
use crate::filesystem::FsError;
use crate::table::Table;

const ABOUT: &str = help_about!("df.md");
Expand Down Expand Up @@ -350,11 +351,25 @@ fn get_all_filesystems(opt: &Options) -> UResult<Vec<Filesystem>> {

// Convert each `MountInfo` into a `Filesystem`, which contains
// both the mount information and usage information.
Ok(mounts
.into_iter()
.filter_map(|m| Filesystem::new(m, None))
.filter(|fs| opt.show_all_fs || fs.usage.blocks > 0)
.collect())
#[cfg(not(windows))]
{
let maybe_mount = |m| Filesystem::from_mount(&mounts, &m, None).ok();
Ok(mounts
.clone()
.into_iter()
.filter_map(maybe_mount)
.filter(|fs| opt.show_all_fs || fs.usage.blocks > 0)
.collect())
}
#[cfg(windows)]
{
let maybe_mount = |m| Filesystem::from_mount(&m, None).ok();
Ok(mounts
.into_iter()
.filter_map(maybe_mount)
.filter(|fs| opt.show_all_fs || fs.usage.blocks > 0)
.collect())
}
}

/// For each path, get the filesystem that contains that path.
Expand Down Expand Up @@ -385,17 +400,25 @@ where
// both the mount information and usage information.
for path in paths {
match Filesystem::from_path(&mounts, path) {
Some(fs) => result.push(fs),
None => {
// this happens if specified file system type != file system type of the file
if path.as_ref().exists() {
show!(USimpleError::new(1, "no file systems processed"));
} else {
show!(USimpleError::new(
1,
format!("{}: No such file or directory", path.as_ref().display())
));
}
Ok(fs) => result.push(fs),
Err(FsError::InvalidPath) => {
show!(USimpleError::new(
1,
format!("{}: No such file or directory", path.as_ref().display())
));
}
Err(FsError::MountMissing) => {
show!(USimpleError::new(1, "no file systems processed"));
}
#[cfg(not(windows))]
Err(FsError::OverMounted) => {
show!(USimpleError::new(
1,
format!(
"cannot access {}: over-mounted by another device",
path.as_ref().quote()
)
));
}
}
}
Expand Down
154 changes: 137 additions & 17 deletions src/uu/df/src/filesystem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,33 @@ pub(crate) struct Filesystem {
pub usage: FsUsage,
}

#[derive(Debug, PartialEq)]
pub(crate) enum FsError {
#[cfg(not(windows))]
OverMounted,
InvalidPath,
MountMissing,
}

/// Check whether `mount` has been over-mounted.
///
/// `mount` is considered over-mounted if it there is an element in
/// `mounts` after mount that has the same mount_dir.
#[cfg(not(windows))]
fn is_over_mounted(mounts: &[MountInfo], mount: &MountInfo) -> bool {
let last_mount_for_dir = mounts
.iter()
.filter(|m| m.mount_dir == mount.mount_dir)
.last();

if let Some(lmi) = last_mount_for_dir {
lmi.dev_name != mount.dev_name
} else {
// Should be unreachable if `mount` is in `mounts`
false
}
}

/// Find the mount info that best matches a given filesystem path.
///
/// This function returns the element of `mounts` on which `path` is
Expand All @@ -56,14 +83,16 @@ fn mount_info_from_path<P>(
path: P,
// This is really only used for testing purposes.
canonicalize: bool,
) -> Option<&MountInfo>
) -> Result<&MountInfo, FsError>
where
P: AsRef<Path>,
{
// TODO Refactor this function with `Stater::find_mount_point()`
// in the `stat` crate.
let path = if canonicalize {
path.as_ref().canonicalize().ok()?
path.as_ref()
.canonicalize()
.map_err(|_| FsError::InvalidPath)?
} else {
path.as_ref().to_path_buf()
};
Expand All @@ -82,12 +111,14 @@ where
.find(|m| m.1.eq(&path))
.map(|m| m.0);

maybe_mount_point.or_else(|| {
mounts
.iter()
.filter(|mi| path.starts_with(&mi.mount_dir))
.max_by_key(|mi| mi.mount_dir.len())
})
maybe_mount_point
.or_else(|| {
mounts
.iter()
.filter(|mi| path.starts_with(&mi.mount_dir))
.max_by_key(|mi| mi.mount_dir.len())
})
.ok_or(FsError::MountMissing)
}

impl Filesystem {
Expand Down Expand Up @@ -117,6 +148,27 @@ impl Filesystem {
})
}

/// Find and create the filesystem from the given mount
/// after checking that the it hasn't been over-mounted
#[cfg(not(windows))]
pub(crate) fn from_mount(
mounts: &[MountInfo],
mount: &MountInfo,
file: Option<String>,
) -> Result<Self, FsError> {
if is_over_mounted(mounts, mount) {
Err(FsError::OverMounted)
} else {
Self::new(mount.clone(), file).ok_or(FsError::MountMissing)
}
}

/// Find and create the filesystem from the given mount.
#[cfg(windows)]
pub(crate) fn from_mount(mount: &MountInfo, file: Option<String>) -> Result<Self, FsError> {
Self::new(mount.clone(), file).ok_or(FsError::MountMissing)
}

/// Find and create the filesystem that best matches a given path.
///
/// This function returns a new `Filesystem` derived from the
Expand All @@ -133,16 +185,18 @@ impl Filesystem {
/// * [`Path::canonicalize`]
/// * [`MountInfo::mount_dir`]
///
pub(crate) fn from_path<P>(mounts: &[MountInfo], path: P) -> Option<Self>
pub(crate) fn from_path<P>(mounts: &[MountInfo], path: P) -> Result<Self, FsError>
where
P: AsRef<Path>,
{
let file = path.as_ref().display().to_string();
let canonicalize = true;
let mount_info = mount_info_from_path(mounts, path, canonicalize)?;
// TODO Make it so that we do not need to clone the `mount_info`.
let mount_info = (*mount_info).clone();
Self::new(mount_info, Some(file))

let result = mount_info_from_path(mounts, path, canonicalize);
#[cfg(windows)]
return result.and_then(|mount_info| Self::from_mount(mount_info, Some(file)));
#[cfg(not(windows))]
return result.and_then(|mount_info| Self::from_mount(mounts, mount_info, Some(file)));
}
}

Expand All @@ -153,7 +207,7 @@ mod tests {

use uucore::fsext::MountInfo;

use crate::filesystem::mount_info_from_path;
use crate::filesystem::{mount_info_from_path, FsError};

// Create a fake `MountInfo` with the given directory name.
fn mount_info(mount_dir: &str) -> MountInfo {
Expand Down Expand Up @@ -183,7 +237,19 @@ mod tests {

#[test]
fn test_empty_mounts() {
assert!(mount_info_from_path(&[], "/", false).is_none());
assert_eq!(
mount_info_from_path(&[], "/", false).unwrap_err(),
FsError::MountMissing
);
}

#[test]
fn test_bad_path() {
assert_eq!(
// This path better not exist....
mount_info_from_path(&[], "/non-existent-path", true).unwrap_err(),
FsError::InvalidPath
);
}

#[test]
Expand All @@ -210,13 +276,19 @@ mod tests {
#[test]
fn test_no_match() {
let mounts = [mount_info("/foo")];
assert!(mount_info_from_path(&mounts, "/bar", false).is_none());
assert_eq!(
mount_info_from_path(&mounts, "/bar", false).unwrap_err(),
FsError::MountMissing
);
}

#[test]
fn test_partial_match() {
let mounts = [mount_info("/foo/bar")];
assert!(mount_info_from_path(&mounts, "/foo/baz", false).is_none());
assert_eq!(
mount_info_from_path(&mounts, "/foo/baz", false).unwrap_err(),
FsError::MountMissing
);
}

#[test]
Expand All @@ -237,4 +309,52 @@ mod tests {
assert!(mount_info_eq(actual, &mounts[0]));
}
}

#[cfg(not(windows))]
mod over_mount {
use crate::filesystem::{is_over_mounted, Filesystem, FsError};
use uucore::fsext::MountInfo;

fn mount_info_with_dev_name(mount_dir: &str, dev_name: Option<&str>) -> MountInfo {
MountInfo {
dev_id: Default::default(),
dev_name: dev_name.map(String::from).unwrap_or_default(),
fs_type: Default::default(),
mount_dir: String::from(mount_dir),
mount_option: Default::default(),
mount_root: Default::default(),
remote: Default::default(),
dummy: Default::default(),
}
}

#[test]
fn test_over_mount() {
let mount_info1 = mount_info_with_dev_name("/foo", Some("dev_name_1"));
let mount_info2 = mount_info_with_dev_name("/foo", Some("dev_name_2"));
let mounts = [mount_info1, mount_info2];
assert!(is_over_mounted(&mounts, &mounts[0]));
}

#[test]
fn test_over_mount_not_over_mounted() {
let mount_info1 = mount_info_with_dev_name("/foo", Some("dev_name_1"));
let mount_info2 = mount_info_with_dev_name("/foo", Some("dev_name_2"));
let mounts = [mount_info1, mount_info2];
assert!(!is_over_mounted(&mounts, &mounts[1]));
}

#[test]
fn test_from_mount_over_mounted() {
let mount_info1 = mount_info_with_dev_name("/foo", Some("dev_name_1"));
let mount_info2 = mount_info_with_dev_name("/foo", Some("dev_name_2"));

let mounts = [mount_info1, mount_info2];

assert_eq!(
Filesystem::from_mount(&mounts, &mounts[0], None).unwrap_err(),
FsError::OverMounted
);
}
}
}
Loading