* [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68
@ 2014-10-29 14:09 Andrew Cooper
2014-10-29 16:03 ` Olaf Hering
2014-11-04 10:48 ` Ian Campbell
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-10-29 14:09 UTC (permalink / raw)
To: Xen-devel; +Cc: Olaf Hering, Wei Liu, Ian Campbell, Andrew Cooper, Ian Jackson
In addition, use os.makedirs() which will also create intermediate directories
if they don't exist.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Olaf Hering <olaf@aepfle.de>
---
tools/pygrub/src/pygrub | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 1ae34a2..37dde32 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -13,7 +13,7 @@
# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
#
-import os, sys, string, struct, tempfile, re, traceback, stat
+import os, sys, string, struct, tempfile, re, traceback, stat, errno
import copy
import logging
import platform
@@ -846,8 +846,14 @@ if __name__ == "__main__":
if debug:
logging.basicConfig(level=logging.DEBUG)
- if not os.path.isdir(output_directory):
- os.mkdir(output_directory, 0700)
+
+ try:
+ os.makedirs(output_directory, 0700)
+ except OSError,e:
+ if (e.errno == errno.EEXIST) and os.path.isdir(output_directory):
+ pass
+ else:
+ raise
if output is None or output == "-":
fd = sys.stdout.fileno()
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68
2014-10-29 14:09 [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68 Andrew Cooper
@ 2014-10-29 16:03 ` Olaf Hering
2014-10-29 18:09 ` Andrew Cooper
2014-11-04 10:48 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Olaf Hering @ 2014-10-29 16:03 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Ian Campbell, Xen-devel
On Wed, Oct 29, Andrew Cooper wrote:
> In addition, use os.makedirs() which will also create intermediate directories
> if they don't exist.
Can this happen in practice, given that /var/run/xen is created by the
runlevel scripts already? The mkdir is now really just @XEN_RUN_DIR@/pygrub.
Olaf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68
2014-10-29 16:03 ` Olaf Hering
@ 2014-10-29 18:09 ` Andrew Cooper
2014-10-29 19:43 ` Olaf Hering
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-10-29 18:09 UTC (permalink / raw)
To: Olaf Hering; +Cc: Wei Liu, Ian Jackson, Ian Campbell, Xen-devel
On 29/10/14 16:03, Olaf Hering wrote:
> On Wed, Oct 29, Andrew Cooper wrote:
>
>> In addition, use os.makedirs() which will also create intermediate directories
>> if they don't exist.
> Can this happen in practice, given that /var/run/xen is created by the
> runlevel scripts already? The mkdir is now really just @XEN_RUN_DIR@/pygrub.
>
> Olaf
Yes, although I presume not given the runlevel scripts.
XenServer, and Xapi in particular, is very different from an upstream xl
based system. It has traditionally had its own initscripts. We are in
the process of disentangling these, but it is not a trivial amount of work.
The result is that I have positively proved that XenServer hit this race
during automatic testing. I agree that it shouldn't happen in reality
for a system configured as expected, but that fact alone doesn't
invalidate the fix itself.
~Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68
2014-10-29 18:09 ` Andrew Cooper
@ 2014-10-29 19:43 ` Olaf Hering
0 siblings, 0 replies; 6+ messages in thread
From: Olaf Hering @ 2014-10-29 19:43 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Ian Campbell, Xen-devel
On Wed, Oct 29, Andrew Cooper wrote:
> The result is that I have positively proved that XenServer hit this race
> during automatic testing. I agree that it shouldn't happen in reality
> for a system configured as expected, but that fact alone doesn't
> invalidate the fix itself.
I think it can happen even with upstream if two guests start at the same
time and both execute the code at the same time. Both will find the dir
does not exist yet and both will attempt to create the dir. I think one
of both will get an exception.
Olaf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68
2014-10-29 14:09 [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68 Andrew Cooper
2014-10-29 16:03 ` Olaf Hering
@ 2014-11-04 10:48 ` Ian Campbell
2014-11-04 10:52 ` Please *justify* your use of the for-4.5 tag when posting patches Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2014-11-04 10:48 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Olaf Hering, Ian Jackson, Xen-devel
On Wed, 2014-10-29 at 14:09 +0000, Andrew Cooper wrote:
> In addition, use os.makedirs() which will also create intermediate directories
> if they don't exist.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Olaf Hering <olaf@aepfle.de>
Acked and applied as a bug fix.
In the future when you include a "for-4.5" tag on a patch please
*always* state your reasoning (usually after the "---"). Don't make the
maintainer / release manager / committer guess why you think it should
be in 4.5. If you consider it "obviously a bug fix" then say so and then
say something about the impact of the bug and the risk associated with
the fix.
I'm a bit grumpy about the number of patches which don't do this, such
patches are mostly going to the back of my queue, alongside the 4.6
patches, since without any rationale that is effectively what I consider
them to be despite the use of the for-4.5 tag.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Please *justify* your use of the for-4.5 tag when posting patches
2014-11-04 10:48 ` Ian Campbell
@ 2014-11-04 10:52 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2014-11-04 10:52 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Olaf Hering, Wei Liu, Andrew Cooper
On Tue, 2014-11-04 at 10:48 +0000, Ian Campbell wrote:
Reposting with a more eyecatching subject, since this was by far not the
only patch with this issue.
> In the future when you include a "for-4.5" tag on a patch please
> *always* state your reasoning (usually after the "---"). Don't make the
> maintainer / release manager / committer guess why you think it should
> be in 4.5. If you consider it "obviously a bug fix" then say so and then
> say something about the impact of the bug and the risk associated with
> the fix.
>
> I'm a bit grumpy about the number of patches which don't do this, such
> patches are mostly going to the back of my queue, alongside the 4.6
> patches, since without any rationale that is effectively what I consider
> them to be despite the use of the for-4.5 tag.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-04 10:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-29 14:09 [PATCH for-4.5] tools/pygrub: Fix TOCTOU race introduced by c/s 63dcc68 Andrew Cooper
2014-10-29 16:03 ` Olaf Hering
2014-10-29 18:09 ` Andrew Cooper
2014-10-29 19:43 ` Olaf Hering
2014-11-04 10:48 ` Ian Campbell
2014-11-04 10:52 ` Please *justify* your use of the for-4.5 tag when posting patches Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.