Re: [PATCH 4/4] resolve_ref_unsafe(): close race condition reading loose refs

Michael Haggerty [off-list ref] writes:

One race is still possible and undetected: another process could
change the file from a regular file into a symlink between the call to
lstat and the call to open().  The open() call would silently follow
the symlink and not know that something is wrong.  I don't see a way
to detect this situation without the use of the O_NOFOLLOW option,
which is not portable and is not used elsewhere in our code base.

However, we don't use symlinks anymore, so this situation is unlikely.
And it doesn't appear that treating a symlink as a regular file would
have grave consequences; after all, this is exactly how the code
handles non-relative symlinks.

You could fstat() the fd you got from open(), and verify that it is
still the same inode/device.  That's wasting one syscall per ref for
pretty much everyone, but perhaps if we really cared about this (and I
gather from the above that we don't), we could conditionally use
O_NOFOLLOW if available, otherwise do that fstat().

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

On 06/13/2013 10:22 AM, Thomas Rast wrote:

Michael Haggerty [off-list ref] writes:

quoted

One race is still possible and undetected: another process could
change the file from a regular file into a symlink between the call to
lstat and the call to open().  The open() call would silently follow
the symlink and not know that something is wrong.  I don't see a way
to detect this situation without the use of the O_NOFOLLOW option,
which is not portable and is not used elsewhere in our code base.

However, we don't use symlinks anymore, so this situation is unlikely.
And it doesn't appear that treating a symlink as a regular file would
have grave consequences; after all, this is exactly how the code
handles non-relative symlinks.

You could fstat() the fd you got from open(), and verify that it is
still the same inode/device.  That's wasting one syscall per ref for
pretty much everyone, but perhaps if we really cared about this (and I
gather from the above that we don't), we could conditionally use
O_NOFOLLOW if available, otherwise do that fstat().

Yes, that would work.  For now I think I will not worry about it, but
I'll keep your trick in mind.

Thanks,
Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/