Skip to content
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

detect attempts to overwrite input data #842

Open
jdtournier opened this issue Nov 30, 2016 · 1 comment

Comments

Projects
None yet
1 participant
@jdtournier
Copy link
Member

commented Nov 30, 2016

As discussed on this community forum thread.

I don't think there's any simple portable way of querying whether a file we've just opened read-write was already being accessed read-only - that would be the ideal scenario. But I do think we can keep track of this ourselves relatively easily, since 99% of file access will be via the File::MMap class - at least for images. That uses a standard C open() call, which returns an integer file descriptor. I think this call is supposed to return the same fd when opening the same file again. So if we keep a list of file descriptors that have previously been opened read-only, we can check when opening a file read-write whether we had already opened it read-only. We could also check whether we had already opened it read-write, but that's less likely to be a problem.

This should sort out most issues with trying to write to one of our inputs, although there will still need to be some changes to some of the image formats that open the file using C++ IO streams (e.g. the MRtrix formats), since it's basically impossible to retrieve the associated fd from that - at least not in a portable way (see this page to get an idea of how complicated that might turn out to be...). The simplest thing to do in these cases will be to enforce that all file access is done via standard C calls that operate on file descriptors (not even FILE* pointers)...

An alternative approach which might actually work more seamlessly is to rely on the device ID & inode number for the file as retrieved via the stat() call - performed in the File::MMap constructor. That combination should be unique to that specific file (have to check what happens with hard or soft links), so all we'd need to do is keep track of those entries that have already been opened read-only - something that should be achievable even if using C++ streams. But we'd still need to catch any uses of std::ifstream and make sure they record these bits of information...

So not trivial, but potentially doable...

@jdtournier

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

Worth checking whether using a MMAP_PRIVATE mapping for the read-only input file fixes the issue - at least on platforms where mmap() is available. Will need to check whether Windows/MSYS2 support equivalent behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.