Skip to content

Commit a1cabac

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 e2872a3 commit a1cabac

File tree

4 files changed

+140
-27
lines changed

4 files changed

+140
-27
lines changed

‎src/diagnostics.rs‎

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

334334
let (title, diag_level) = match e {

‎src/helpers.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
409409
RejectOpWith::WarningWithoutBacktrace => {
410410
this.tcx
411411
.sess
412-
.warn(&format!("`{}` was made to return an error due to isolation", op_name));
412+
.warn(&format!("{} was made to return an error due to isolation", op_name));
413413
Ok(())
414414
}
415415
RejectOpWith::Warning => {

‎src/shims/env.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
322322
let size = this.read_scalar(&size_op)?.to_machine_usize(&*this.tcx)?;
323323

324324
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
325-
this.reject_in_isolation("getcwd", reject_with)?;
325+
this.reject_in_isolation("`getcwd`", reject_with)?;
326326
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
327327
return Ok(Pointer::null());
328328
}
@@ -355,7 +355,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
355355
let buf = this.read_pointer(buf_op)?;
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
}
@@ -380,7 +380,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
380380
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
381381

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

386386
return Ok(-1);
@@ -408,7 +408,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
408408
let path = this.read_path_from_wide_str(this.read_pointer(path_op)?)?;
409409

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

414414
return Ok(0);

‎src/shims/posix/fs.rs‎

Lines changed: 134 additions & 21 deletions
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_pointer(path_op)?)?;
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_pointer(target_op)?)?;
817837
let linkpath = this.read_path_from_c_str(this.read_pointer(linkpath_op)?)?;
@@ -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_ptr = this.read_pointer(statxbuf_op)?;
880929
let pathname_ptr = this.read_pointer(pathname_op)?;
@@ -1032,7 +1081,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
10321081
) -> InterpResult<'tcx, i32> {
10331082
let this = self.eval_context_mut();
10341083

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

10371091
let oldpath_ptr = this.read_pointer(oldpath_op)?;
10381092
let newpath_ptr = this.read_pointer(newpath_op)?;
@@ -1058,7 +1112,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
10581112
) -> InterpResult<'tcx, i32> {
10591113
let this = self.eval_context_mut();
10601114

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

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

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

10931157
let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
10941158

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

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

11051175
let name = this.read_path_from_c_str(this.read_pointer(name_op)?)?;
11061176

@@ -1131,7 +1201,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
11311201
let this = self.eval_context_mut();
11321202

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

11361212
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
11371213

@@ -1224,7 +1300,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
12241300
let this = self.eval_context_mut();
12251301

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

12291311
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
12301312

@@ -1313,7 +1395,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13131395
fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13141396
let this = self.eval_context_mut();
13151397

1316-
this.check_no_isolation("`closedir`")?;
1398+
// Reject if isolation is enabled.
1399+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1400+
this.reject_in_isolation("`closedir`", reject_with)?;
1401+
// Set error code as "EBADF" (bad fd)
1402+
return this.handle_not_found();
1403+
}
13171404

13181405
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
13191406

@@ -1332,7 +1419,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13321419
) -> InterpResult<'tcx, i32> {
13331420
let this = self.eval_context_mut();
13341421

1335-
this.check_no_isolation("`ftruncate64`")?;
1422+
// Reject if isolation is enabled.
1423+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1424+
this.reject_in_isolation("`ftruncate64`", reject_with)?;
1425+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1426+
return Ok(-1);
1427+
}
13361428

13371429
let fd = this.read_scalar(fd_op)?.to_i32()?;
13381430
let length = this.read_scalar(length_op)?.to_i64()?;
@@ -1367,7 +1459,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13671459

13681460
let this = self.eval_context_mut();
13691461

1370-
this.check_no_isolation("`fsync`")?;
1462+
// Reject if isolation is enabled.
1463+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1464+
this.reject_in_isolation("`fsync`", reject_with)?;
1465+
// Set error code as "EBADF" (bad fd)
1466+
return this.handle_not_found();
1467+
}
13711468

13721469
let fd = this.read_scalar(fd_op)?.to_i32()?;
13731470
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1383,7 +1480,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13831480
fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13841481
let this = self.eval_context_mut();
13851482

1386-
this.check_no_isolation("`fdatasync`")?;
1483+
// Reject if isolation is enabled.
1484+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1485+
this.reject_in_isolation("`fdatasync`", reject_with)?;
1486+
// Set error code as "EBADF" (bad fd)
1487+
return this.handle_not_found();
1488+
}
13871489

13881490
let fd = this.read_scalar(fd_op)?.to_i32()?;
13891491
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1405,7 +1507,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
14051507
) -> InterpResult<'tcx, i32> {
14061508
let this = self.eval_context_mut();
14071509

1408-
this.check_no_isolation("`sync_file_range`")?;
1510+
// Reject if isolation is enabled.
1511+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1512+
this.reject_in_isolation("`sync_file_range`", reject_with)?;
1513+
// Set error code as "EBADF" (bad fd)
1514+
return this.handle_not_found();
1515+
}
14091516

14101517
let fd = this.read_scalar(fd_op)?.to_i32()?;
14111518
let offset = this.read_scalar(offset_op)?.to_i64()?;
@@ -1444,7 +1551,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
14441551
) -> InterpResult<'tcx, i64> {
14451552
let this = self.eval_context_mut();
14461553

1447-
this.check_no_isolation("readlink")?;
1554+
// Reject if isolation is enabled.
1555+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1556+
this.reject_in_isolation("`readlink`", reject_with)?;
1557+
// readlink never sets "EPERM". Set "ENOENT".
1558+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
1559+
return Ok(-1);
1560+
}
14481561

14491562
let pathname = this.read_path_from_c_str(this.read_pointer(pathname_op)?)?;
14501563
let buf = this.read_pointer(buf_op)?;

0 commit comments

Comments
 (0)