-
-
Notifications
You must be signed in to change notification settings - Fork 2k
chroot: report ENAMETOOLONG instead of "no such directory" for long NEWROOT #13881
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,6 +194,14 @@ | |
| options.chroot_target = resolved; | ||
| } | ||
|
|
||
| // Probe the path with `metadata` so the *original* I/O error reaches the | ||
| // user. `Path::is_dir()` swallows that error and reports `false` for any | ||
| // failure — including `ENAMETOOLONG` (a 256-char path on Linux), which GNU | ||
| // surfaces as "File name too long". Without this, a too-long NEWROOT | ||
| // would be misreported as "no such directory". | ||
| if let Err(e) = std::fs::metadata(&options.newroot) { | ||
| return Err(ChrootError::CannotStat(options.newroot, e).into()); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should remove .is_dir() on many platforms instead of adding such hack to extract err from ctual chroot call instead. |
||
| if !options.newroot.is_dir() { | ||
| return Err(ChrootError::NoSuchDirectory(options.newroot).into()); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,10 @@ | |
| #[error("{}", translate!("chroot-error-no-such-directory", "dir" => _0.quote()))] | ||
| NoSuchDirectory(PathBuf), | ||
|
|
||
| /// The given path could not be stat'd (e.g. ENAMETOOLONG, EACCES). | ||
| #[error("{}", translate!("chroot-error-cannot-stat", "dir" => _0.quote(), "err" => _1))] | ||
| CannotStat(PathBuf, #[source] Error), | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed even it is extracted from OS?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened a better alternative #13881 . |
||
| /// The call to `setgid()` failed. | ||
| #[error("{}", translate!("chroot-error-set-gid-failed", "gid" => _0, "err" => _1))] | ||
| SetGidFailed(String, #[source] Error), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,22 @@ | |
| .stderr_is("chroot: cannot change root directory to 'a': no such directory\n"); | ||
| } | ||
|
|
||
| #[test] | ||
| #[cfg(not(target_os = "android"))] | ||
| fn test_filename_too_long() { | ||
| // Regression for #13156: a NEWROOT longer than NAME_MAX (255 bytes on | ||
| // Linux) used to be misreported as "no such directory" because | ||
| // `Path::is_dir()` swallows ENAMETOOLONG. GNU surfaces the real error. | ||
| let long_name = "A".repeat(256); | ||
| let expected = format!( | ||
| "chroot: cannot change root directory to '{long_name}': File name too long (os error 63)\n" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use the strip_errno in the code to remove the (os error 63) |
||
| ); | ||
| new_ucmd!() | ||
| .arg(&long_name) | ||
| .fails_with_code(125) | ||
| .stderr_is(expected); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_multiple_group_args() { | ||
| let ts = TestScenario::new(util_name!()); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make the comment shorter