Skip to content

Commit 9d413ea

Browse files
atsmtatatsmtat
committed
Fix use of deprecated check_no_isolation in posix fs ops
Update posix fs shims to use new API `reject_in_isolation`, which allows rejection with error code instead of always forcing abort. Error code chosen for each op is the most appropriate one from the list in corresponding syscall's manual. Updated helper APIs to not use quotes (`) around input name while preparing the message. This allows callers to pass multi-word string like -- "`read` from stdin".
1 parent 35af23b commit 9d413ea

File tree

4 files changed

+140
-27
lines changed

4 files changed

+140
-27
lines changed

‎src/diagnostics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
327327
CreatedAlloc(AllocId(id)) => format!("created allocation with id {}", id),
328328
FreedAlloc(AllocId(id)) => format!("freed allocation with id {}", id),
329329
RejectedIsolatedOp(ref op) =>
330-
format!("`{}` was made to return an error due to isolation", op),
330+
format!("{} was made to return an error due to isolation", op),
331331
};
332332

333333
let (title, diag_level) = match e {

‎src/helpers.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
407407
RejectOpWith::WarningWithoutBacktrace => {
408408
this.tcx
409409
.sess
410-
.warn(&format!("`{}` was made to return an error due to isolation", op_name));
410+
.warn(&format!("{} was made to return an error due to isolation", op_name));
411411
Ok(())
412412
}
413413
RejectOpWith::Warning => {

‎src/shims/env.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
323323
);
324324

325325
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
326-
this.reject_in_isolation("getcwd", reject_with)?;
326+
this.reject_in_isolation("`getcwd`", reject_with)?;
327327
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
328328
return Ok(Scalar::null_ptr(&*this.tcx));
329329
}
@@ -355,7 +355,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
355355
this.assert_target_os("windows", "GetCurrentDirectoryW");
356356

357357
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
358-
this.reject_in_isolation("GetCurrentDirectoryW", reject_with)?;
358+
this.reject_in_isolation("`GetCurrentDirectoryW`", reject_with)?;
359359
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
360360
return Ok(0);
361361
}
@@ -381,7 +381,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
381381
);
382382

383383
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
384-
this.reject_in_isolation("chdir", reject_with)?;
384+
this.reject_in_isolation("`chdir`", reject_with)?;
385385
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
386386

387387
return Ok(-1);
@@ -409,7 +409,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
409409
this.assert_target_os("windows", "SetCurrentDirectoryW");
410410

411411
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
412-
this.reject_in_isolation("SetCurrentDirectoryW", reject_with)?;
412+
this.reject_in_isolation("`SetCurrentDirectoryW`", reject_with)?;
413413
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
414414

415415
return Ok(0);

‎src/shims/posix/fs.rs

+134-21
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::convert::{TryFrom, TryInto};
44
use std::fs::{
55
read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir,
66
};
7-
use std::io::{self, Read, Seek, SeekFrom, Write};
7+
use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write};
88
use std::path::Path;
99
use std::time::SystemTime;
1010

@@ -504,7 +504,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
504504
) -> InterpResult<'tcx, i32> {
505505
let this = self.eval_context_mut();
506506

507-
this.check_no_isolation("`open`")?;
507+
// Reject if isolation is enabled.
508+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
509+
this.reject_in_isolation("`open`", reject_with)?;
510+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
511+
return Ok(-1);
512+
}
508513

509514
let flag = this.read_scalar(flag_op)?.to_i32()?;
510515

@@ -594,7 +599,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
594599
fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> {
595600
let this = self.eval_context_mut();
596601

597-
this.check_no_isolation("`fcntl`")?;
602+
// Reject if isolation is enabled.
603+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
604+
this.reject_in_isolation("`fcntl`", reject_with)?;
605+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
606+
return Ok(-1);
607+
}
598608

599609
if args.len() < 2 {
600610
throw_ub_format!(
@@ -785,7 +795,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
785795
fn unlink(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
786796
let this = self.eval_context_mut();
787797

788-
this.check_no_isolation("`unlink`")?;
798+
// Reject if isolation is enabled.
799+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
800+
this.reject_in_isolation("`unlink`", reject_with)?;
801+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
802+
return Ok(-1);
803+
}
789804

790805
let path = this.read_path_from_c_str(this.read_scalar(path_op)?.check_init()?)?;
791806

@@ -811,7 +826,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
811826

812827
let this = self.eval_context_mut();
813828

814-
this.check_no_isolation("`symlink`")?;
829+
// Reject if isolation is enabled.
830+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
831+
this.reject_in_isolation("`symlink`", reject_with)?;
832+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
833+
return Ok(-1);
834+
}
815835

816836
let target = this.read_path_from_c_str(this.read_scalar(target_op)?.check_init()?)?;
817837
let linkpath = this.read_path_from_c_str(this.read_scalar(linkpath_op)?.check_init()?)?;
@@ -827,7 +847,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
827847
) -> InterpResult<'tcx, i32> {
828848
let this = self.eval_context_mut();
829849
this.assert_target_os("macos", "stat");
830-
this.check_no_isolation("`stat`")?;
850+
851+
// Reject if isolation is enabled.
852+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
853+
this.reject_in_isolation("`stat`", reject_with)?;
854+
// macos stat never sets "EPERM". Set error code "ENOENT".
855+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
856+
return Ok(-1);
857+
}
858+
831859
// `stat` always follows symlinks.
832860
this.macos_stat_or_lstat(true, path_op, buf_op)
833861
}
@@ -840,7 +868,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
840868
) -> InterpResult<'tcx, i32> {
841869
let this = self.eval_context_mut();
842870
this.assert_target_os("macos", "lstat");
843-
this.check_no_isolation("`lstat`")?;
871+
872+
// Reject if isolation is enabled.
873+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
874+
this.reject_in_isolation("`lstat`", reject_with)?;
875+
// macos lstat never sets "EPERM". Set error code "ENOENT".
876+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
877+
return Ok(-1);
878+
}
879+
844880
this.macos_stat_or_lstat(false, path_op, buf_op)
845881
}
846882

@@ -852,7 +888,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
852888
let this = self.eval_context_mut();
853889

854890
this.assert_target_os("macos", "fstat");
855-
this.check_no_isolation("`fstat`")?;
891+
892+
// Reject if isolation is enabled.
893+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
894+
this.reject_in_isolation("`fstat`", reject_with)?;
895+
// Set error code as "EBADF" (bad fd)
896+
return this.handle_not_found();
897+
}
856898

857899
let fd = this.read_scalar(fd_op)?.to_i32()?;
858900

@@ -874,7 +916,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
874916
let this = self.eval_context_mut();
875917

876918
this.assert_target_os("linux", "statx");
877-
this.check_no_isolation("`statx`")?;
919+
920+
// Reject if isolation is enabled.
921+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
922+
this.reject_in_isolation("`statx`", reject_with)?;
923+
// statx never sets "EPERM". Set error code "ENOENT".
924+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
925+
return Ok(-1);
926+
}
878927

879928
let statxbuf_scalar = this.read_scalar(statxbuf_op)?.check_init()?;
880929
let pathname_scalar = this.read_scalar(pathname_op)?.check_init()?;
@@ -1034,7 +1083,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
10341083
) -> InterpResult<'tcx, i32> {
10351084
let this = self.eval_context_mut();
10361085

1037-
this.check_no_isolation("`rename`")?;
1086+
// Reject if isolation is enabled.
1087+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1088+
this.reject_in_isolation("`rename`", reject_with)?;
1089+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1090+
return Ok(-1);
1091+
}
10381092

10391093
let oldpath_scalar = this.read_scalar(oldpath_op)?.check_init()?;
10401094
let newpath_scalar = this.read_scalar(newpath_op)?.check_init()?;
@@ -1060,7 +1114,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
10601114
) -> InterpResult<'tcx, i32> {
10611115
let this = self.eval_context_mut();
10621116

1063-
this.check_no_isolation("`mkdir`")?;
1117+
// Reject if isolation is enabled.
1118+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1119+
this.reject_in_isolation("`mkdir`", reject_with)?;
1120+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1121+
return Ok(-1);
1122+
}
10641123

10651124
#[cfg_attr(not(unix), allow(unused_variables))]
10661125
let mode = if this.tcx.sess.target.os == "macos" {
@@ -1090,7 +1149,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
10901149
fn rmdir(&mut self, path_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
10911150
let this = self.eval_context_mut();
10921151

1093-
this.check_no_isolation("`rmdir`")?;
1152+
// Reject if isolation is enabled.
1153+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1154+
this.reject_in_isolation("`rmdir`", reject_with)?;
1155+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1156+
return Ok(-1);
1157+
}
10941158

10951159
let path = this.read_path_from_c_str(this.read_scalar(path_op)?.check_init()?)?;
10961160

@@ -1102,7 +1166,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
11021166
fn opendir(&mut self, name_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Tag>> {
11031167
let this = self.eval_context_mut();
11041168

1105-
this.check_no_isolation("`opendir`")?;
1169+
// Reject if isolation is enabled.
1170+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1171+
this.reject_in_isolation("`opendir`", reject_with)?;
1172+
// opendir function never sets "EPERM". Set "ENOENT".
1173+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
1174+
return Ok(Scalar::null_ptr(this));
1175+
}
11061176

11071177
let name = this.read_path_from_c_str(this.read_scalar(name_op)?.check_init()?)?;
11081178

@@ -1133,7 +1203,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
11331203
let this = self.eval_context_mut();
11341204

11351205
this.assert_target_os("linux", "readdir64_r");
1136-
this.check_no_isolation("`readdir64_r`")?;
1206+
1207+
// Reject if isolation is enabled.
1208+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1209+
this.reject_in_isolation("`readdir64_r`", reject_with)?;
1210+
// Set error code as "EBADF" (bad fd)
1211+
return this.handle_not_found();
1212+
}
11371213

11381214
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
11391215

@@ -1222,7 +1298,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
12221298
let this = self.eval_context_mut();
12231299

12241300
this.assert_target_os("macos", "readdir_r");
1225-
this.check_no_isolation("`readdir_r`")?;
1301+
1302+
// Reject if isolation is enabled.
1303+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1304+
this.reject_in_isolation("`readdir_r`", reject_with)?;
1305+
// Set error code as "EBADF" (bad fd)
1306+
return this.handle_not_found();
1307+
}
12261308

12271309
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
12281310

@@ -1307,7 +1389,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13071389
fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13081390
let this = self.eval_context_mut();
13091391

1310-
this.check_no_isolation("`closedir`")?;
1392+
// Reject if isolation is enabled.
1393+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1394+
this.reject_in_isolation("`closedir`", reject_with)?;
1395+
// Set error code as "EBADF" (bad fd)
1396+
return this.handle_not_found();
1397+
}
13111398

13121399
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
13131400

@@ -1326,7 +1413,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13261413
) -> InterpResult<'tcx, i32> {
13271414
let this = self.eval_context_mut();
13281415

1329-
this.check_no_isolation("`ftruncate64`")?;
1416+
// Reject if isolation is enabled.
1417+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1418+
this.reject_in_isolation("`ftruncate64`", reject_with)?;
1419+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1420+
return Ok(-1);
1421+
}
13301422

13311423
let fd = this.read_scalar(fd_op)?.to_i32()?;
13321424
let length = this.read_scalar(length_op)?.to_i64()?;
@@ -1361,7 +1453,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13611453

13621454
let this = self.eval_context_mut();
13631455

1364-
this.check_no_isolation("`fsync`")?;
1456+
// Reject if isolation is enabled.
1457+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1458+
this.reject_in_isolation("`fsync`", reject_with)?;
1459+
// Set error code as "EBADF" (bad fd)
1460+
return this.handle_not_found();
1461+
}
13651462

13661463
let fd = this.read_scalar(fd_op)?.to_i32()?;
13671464
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1377,7 +1474,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13771474
fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13781475
let this = self.eval_context_mut();
13791476

1380-
this.check_no_isolation("`fdatasync`")?;
1477+
// Reject if isolation is enabled.
1478+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1479+
this.reject_in_isolation("`fdatasync`", reject_with)?;
1480+
// Set error code as "EBADF" (bad fd)
1481+
return this.handle_not_found();
1482+
}
13811483

13821484
let fd = this.read_scalar(fd_op)?.to_i32()?;
13831485
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1399,7 +1501,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13991501
) -> InterpResult<'tcx, i32> {
14001502
let this = self.eval_context_mut();
14011503

1402-
this.check_no_isolation("`sync_file_range`")?;
1504+
// Reject if isolation is enabled.
1505+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1506+
this.reject_in_isolation("`sync_file_range`", reject_with)?;
1507+
// Set error code as "EBADF" (bad fd)
1508+
return this.handle_not_found();
1509+
}
14031510

14041511
let fd = this.read_scalar(fd_op)?.to_i32()?;
14051512
let offset = this.read_scalar(offset_op)?.to_i64()?;
@@ -1438,7 +1545,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
14381545
) -> InterpResult<'tcx, i64> {
14391546
let this = self.eval_context_mut();
14401547

1441-
this.check_no_isolation("readlink")?;
1548+
// Reject if isolation is enabled.
1549+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1550+
this.reject_in_isolation("`readlink`", reject_with)?;
1551+
// readlink never sets "EPERM". Set "ENOENT".
1552+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
1553+
return Ok(-1);
1554+
}
14421555

14431556
let pathname = this.read_path_from_c_str(this.read_scalar(pathname_op)?.check_init()?)?;
14441557
let buf = this.read_scalar(buf_op)?.check_init()?;

0 commit comments

Comments
 (0)