You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2004/09/05 23:44:22 UTC
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
bliss@tigris.org wrote:
>Author: bliss
>Date: Sun Sep 5 10:32:02 2004
>New Revision: 10819
>
>Modified:
> trunk/subversion/libsvn_subr/cmdline.c
>Log:
>Prevent severe database corruption and other horrors on unix systems
>if e.g. svnadmin is run with it's standard input, standard output or
>standard error file descriptors closed.
>
>* subversion/libsvn_subr/cmdline.c
> (svn_cmdline_init): Open '/dev/null' for standard input, output and
> error if they are closed.
>
>
I'd like to know why this code should be Unix-specific. I can't see a
single reason.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2004-09-06 at 01:41, Max Bowsher wrote:
> Nevertheless, I wonder whether we should enable the code on Windows anyway,
> in case there is some strange set of circumstances that can cause the
> problem. Three stat() calls shouldn't be vastly expensive, after all.
I don't think so. This really sounds like a Unix-specific problem (in
that only Unix allows the start condition which leads to the file
corruption) and it's clearer to have a Unix-specific solution.
Especially since we try avoiding the fd-level interface on Windows
altogether. (I'm not saying that avoidance saves us from the problem,
since our dependency libraries might use it, but it does give us a
reason to avoid making fd-level calls on Windows in order to solve a
non-problem.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Max Bowsher <ma...@ukf.net>.
Greg Hudson wrote:
> On Sun, 2004-09-05 at 19:52, Branko ÄŒibej wrote:
>>> I didn't think Windows really had the concept of file descriptors
>>> outside of the POSIX terarium, much less file descriptors which (by
>>> virtue of their numbers) map onto stdin/stdout/stderr.
>
>> Windows has all of that, even though it's simulated by the C runtime
>> library. That's why I'd like to see proof (either through testing, or
>> BDB code review) that this problem really doesn't occur on Windows
...
> On Unix, the bindings of fds 0/1/2 at process startup are purely in the
> hands of the calling process; the fork() and exec() calls don't care
> about them. In the Windows world,
> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/createprocess.asp
> and
> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/startupinfo_str.asp
> suggest that there is much more structure involved. I don't know what
> happens if you pass INVALID_HANDLE_VALID in the startup info structure,
> but it seems likely that Windows will do something intelligent (either
> mapping that file handle to the console, or mapping it to a null
> device).
I ran some tests.
Within a single process, it is possible to close standard fds, and have a
subsequent open() return 0/1/2.
However, if you exec a subprocess, open()-ed fds start from 3.
Also, despite playing around with several invocations of CreateProcess, I
was unable create a situation without preopened 0/1/2.
Nevertheless, I wonder whether we should enable the code on Windows anyway,
in case there is some strange set of circumstances that can cause the
problem. Three stat() calls shouldn't be vastly expensive, after all.
Max.
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
Branko Čibej wrote:
> Greg Hudson wrote:
>
>> BDB code review? I'm not sure how that could be relevant.
>
> The problem shows up when a write to stdout finds its way into a file
> opened by BDB, doesn't it? If BDB opens files differently on Windows
> than it does on Unix, we're safe.
I just happens to be BDB in a lot of cases, but it could be any file. It
depends on the order in which files are opened. If you're using fsfs,
you'll probably corrupt an fsfs file. If you're using a remote command,
you'll probably write garbage (i.e. intended stdout/stderr output) to
the socket, etc.
> Handles aren't really the issue here, because there are no
> "well-known" handle values for the standard streams. But the CRT does
> provide a file-descriptor-like interface where FD's 0, 1, and 2 _are_
> special, and the standard C FILE* streams stdin, stdout and stderr are
> bound to those. Those streams can be closed just like on Unix, and
> from all I've seen, stream slots are assigned sequentially. So we
> could potentially be exposed to the same problem on Windows.
Max's experiments indicates that a new process will always have fds
0/1/2 open, which means that we don't have this problem on Win32. On the
other hand, it's better to be safe than sorry, so if someone with a
Win32 build environment would like to adapt the fix to WIN32, it's
probably a good idea, but unless we can have such a fix really soon, I
don't think we should delay the fix for unix. There's nothing preventing
a Win32 fix from going in later.
/Tobias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:
>On Sun, 2004-09-05 at 19:52, Branko ÄŒibej wrote:
>
>
>>>I didn't think Windows really had the concept of file descriptors
>>>outside of the POSIX terarium, much less file descriptors which (by
>>>virtue of their numbers) map onto stdin/stdout/stderr.
>>>
>>>
>
>
>
>>Windows has all of that, even though it's simulated by the C runtime
>>library. That's why I'd like to see proof (either through testing, or
>>BDB code review) that this problem really doesn't occur on Windows
>>
>>
>
>BDB code review? I'm not sure how that could be relevant.
>
>
The problem shows up when a write to stdout finds its way into a file
opened by BDB, doesn't it? If BDB opens files differently on Windows
than it does on Unix, we're safe.
>On Unix, the bindings of fds 0/1/2 at process startup are purely in the
>hands of the calling process; the fork() and exec() calls don't care
>about them. In the Windows world,
>http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/createprocess.asp
>and
>http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/startupinfo_str.asp
>suggest that there is much more structure involved. I don't know what
>happens if you pass INVALID_HANDLE_VALID in the startup info structure,
>but it seems likely that Windows will do something intelligent (either
>mapping that file handle to the console, or mapping it to a null
>device).
>
Handles aren't really the issue here, because there are no "well-known"
handle values for the standard streams. But the CRT does provide a
file-descriptor-like interface where FD's 0, 1, and 2 _are_ special, and
the standard C FILE* streams stdin, stdout and stderr are bound to
those. Those streams can be closed just like on Unix, and from all I've
seen, stream slots are assigned sequentially. So we could potentially be
exposed to the same problem on Windows.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-09-05 at 19:52, Branko Čibej wrote:
> >I didn't think Windows really had the concept of file descriptors
> >outside of the POSIX terarium, much less file descriptors which (by
> >virtue of their numbers) map onto stdin/stdout/stderr.
> Windows has all of that, even though it's simulated by the C runtime
> library. That's why I'd like to see proof (either through testing, or
> BDB code review) that this problem really doesn't occur on Windows
BDB code review? I'm not sure how that could be relevant.
On Unix, the bindings of fds 0/1/2 at process startup are purely in the
hands of the calling process; the fork() and exec() calls don't care
about them. In the Windows world,
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/createprocess.asp
and
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/startupinfo_str.asp
suggest that there is much more structure involved. I don't know what
happens if you pass INVALID_HANDLE_VALID in the startup info structure,
but it seems likely that Windows will do something intelligent (either
mapping that file handle to the console, or mapping it to a null
device).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:
>On Sun, 2004-09-05 at 19:44, Branko ÄŒibej wrote:
>
>
>>I'd like to know why this code should be Unix-specific. I can't see a
>>single reason.
>>
>>
>
>The code is intended to handle the case where an svn command-line app is
>invoked with no file descriptor 0, 1, or 2,
>
Yup, I know.
> such that the first few
>files opened become accidentally mapped to stdin, stdout, or stderr.
>
>I didn't think Windows really had the concept of file descriptors
>outside of the POSIX terarium, much less file descriptors which (by
>virtue of their numbers) map onto stdin/stdout/stderr.
>
>
Windows has all of that, even though it's simulated by the C runtime
library. That's why I'd like to see proof (either through testing, or
BDB code review) that this problem really doesn't occur on Windows
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10819 - trunk/subversion/libsvn_subr
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-09-05 at 19:44, Branko Čibej wrote:
> I'd like to know why this code should be Unix-specific. I can't see a
> single reason.
The code is intended to handle the case where an svn command-line app is
invoked with no file descriptor 0, 1, or 2, such that the first few
files opened become accidentally mapped to stdin, stdout, or stderr.
I didn't think Windows really had the concept of file descriptors
outside of the POSIX terarium, much less file descriptors which (by
virtue of their numbers) map onto stdin/stdout/stderr.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org