Skip to content

Commit e3b7b82

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 a4a9a36 commit e3b7b82

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
@@ -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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
324324
);
325325

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

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

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

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

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

416416
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

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

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

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

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

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

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

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

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

@@ -812,7 +827,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
812827

813828
let this = self.eval_context_mut();
814829

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

817837
let target = this.read_path_from_c_str(this.read_scalar(target_op)?.check_init()?)?;
818838
let linkpath = this.read_path_from_c_str(this.read_scalar(linkpath_op)?.check_init()?)?;
@@ -828,7 +848,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
828848
) -> InterpResult<'tcx, i32> {
829849
let this = self.eval_context_mut();
830850
this.assert_target_os("macos", "stat");
831-
this.check_no_isolation("`stat`")?;
851+
852+
// Reject if isolation is enabled.
853+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
854+
this.reject_in_isolation("`stat`", reject_with)?;
855+
// macos stat never sets "EPERM". Set error code "ENOENT".
856+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
857+
return Ok(-1);
858+
}
859+
832860
// `stat` always follows symlinks.
833861
this.macos_stat_or_lstat(true, path_op, buf_op)
834862
}
@@ -841,7 +869,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
841869
) -> InterpResult<'tcx, i32> {
842870
let this = self.eval_context_mut();
843871
this.assert_target_os("macos", "lstat");
844-
this.check_no_isolation("`lstat`")?;
872+
873+
// Reject if isolation is enabled.
874+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
875+
this.reject_in_isolation("`lstat`", reject_with)?;
876+
// macos lstat never sets "EPERM". Set error code "ENOENT".
877+
this.set_last_error_from_io_error(ErrorKind::NotFound)?;
878+
return Ok(-1);
879+
}
880+
845881
this.macos_stat_or_lstat(false, path_op, buf_op)
846882
}
847883

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

@@ -1227,7 +1303,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
12271303
let this = self.eval_context_mut();
12281304

12291305
this.assert_target_os("macos", "readdir_r");
1230-
this.check_no_isolation("`readdir_r`")?;
1306+
1307+
// Reject if isolation is enabled.
1308+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1309+
this.reject_in_isolation("`readdir_r`", reject_with)?;
1310+
// Set error code as "EBADF" (bad fd)
1311+
return this.handle_not_found();
1312+
}
12311313

12321314
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
12331315

@@ -1316,7 +1398,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13161398
fn closedir(&mut self, dirp_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13171399
let this = self.eval_context_mut();
13181400

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

13211408
let dirp = this.read_scalar(dirp_op)?.to_machine_usize(this)?;
13221409

@@ -1335,7 +1422,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13351422
) -> InterpResult<'tcx, i32> {
13361423
let this = self.eval_context_mut();
13371424

1338-
this.check_no_isolation("`ftruncate64`")?;
1425+
// Reject if isolation is enabled.
1426+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
1427+
this.reject_in_isolation("`ftruncate64`", reject_with)?;
1428+
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
1429+
return Ok(-1);
1430+
}
13391431

13401432
let fd = this.read_scalar(fd_op)?.to_i32()?;
13411433
let length = this.read_scalar(length_op)?.to_i64()?;
@@ -1370,7 +1462,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13701462

13711463
let this = self.eval_context_mut();
13721464

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

13751472
let fd = this.read_scalar(fd_op)?.to_i32()?;
13761473
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1386,7 +1483,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
13861483
fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
13871484
let this = self.eval_context_mut();
13881485

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

13911493
let fd = this.read_scalar(fd_op)?.to_i32()?;
13921494
if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
@@ -1408,7 +1510,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
14081510
) -> InterpResult<'tcx, i32> {
14091511
let this = self.eval_context_mut();
14101512

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

14131520
let fd = this.read_scalar(fd_op)?.to_i32()?;
14141521
let offset = this.read_scalar(offset_op)?.to_i64()?;
@@ -1447,7 +1554,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
14471554
) -> InterpResult<'tcx, i64> {
14481555
let this = self.eval_context_mut();
14491556

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

14521565
let pathname = this.read_path_from_c_str(this.read_scalar(pathname_op)?.check_init()?)?;
14531566
let buf = this.read_scalar(buf_op)?.check_init()?;

0 commit comments

Comments
 (0)