cleans /tmp/ida every time ida is called #853
Conversation
fix BinaryAnalysisPlatform#720 This PR removes dump files from `/tmp/ida` before any ida launch
There was a problem hiding this comment.
This code doesn't prevent bap from failing because ida dump exists because of TOCTOU bug. Also it may interact with other processes.
You shall create a lock file and cleanup the folder while have an exclusive access to it.
Also, your code is hard to read, for such a simple operation as rm -f /tmp/ida/*.dmp. It shall not be that bad. There is no need to use FileUtil if they do not make your code more readable, plain OCaml is sufficient here,
let tmp = "/tmp/ida in
if Sys.file_exists tmp && Sys.is_directory tmp then
Sys.readdir tmp |> Array.iter ~f:(fun n ->
if Filename.check_suffix n ".dmp" then Sys.remove n)Just add the locking procedure and you are done.
|
Please, look into |
plugins/ida/bap_ida_service.ml
Outdated
| let cleanup_minidump () = | ||
| let is_dump x = String.equal (Filename.extension x) ".dmp" in | ||
| let dump_path = "/tmp/ida" in | ||
| if Sys.file_exists dump_path then |
There was a problem hiding this comment.
it will fail if dump_path is a file, add Sys.is_directory
plugins/ida/bap_ida_service.ml
Outdated
| @@ -79,8 +81,24 @@ let setup_headless_env path = | |||
| if old_path <> "" then Unix.putenv var old_path | |||
| with _ -> () | |||
|
|
|||
| let cleanup_minidump () = | |||
| let is_dump x = String.equal (Filename.extension x) ".dmp" in | |||
There was a problem hiding this comment.
use Filename.check_suffix n ".dmp"
in general use the most specialized function available. It conveys you intentions better and is usually more efficient and straightforward
| | [] -> () | ||
| | files -> | ||
| info "ida minidump is not empty"; | ||
| let lock = sprintf "%s/lock" dump_path in |
There was a problem hiding this comment.
protect this code with finally, to ensure that the file is always unlocked.
plugins/ida/bap_ida_service.ml
Outdated
| | files -> | ||
| info "ida minidump is not empty"; | ||
| let lock = sprintf "%s/lock" dump_path in | ||
| let lock = Unix.openfile lock Unix.[O_RDWR; O_CREAT] 0o640 in |
There was a problem hiding this comment.
since the lockfile is global it should be readable (and thus shareable) by all users, so provide corresponding rights.
Consider the scenario when BAP is run by two different users, the first user will create a lock file, while the second will not be able to graph the lock as she will fail on open.
fix #720
Just remove dump files from
/tmp/idabefore anyidalaunch