Patchwork [STABLE] manifest: fix possible SEGV caused by uninitialized lazymanifest fields

login
register
mail settings
Submitter Yuya Nishihara
Date June 15, 2018, 1:54 p.m.
Message ID <c058c6d1a857dac021d5.1529070899@mimosa>
Download mbox | patch
Permalink /patch/32176/
State Accepted
Headers show

Comments

Yuya Nishihara - June 15, 2018, 1:54 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1529068618 -32400
#      Fri Jun 15 22:16:58 2018 +0900
# Branch stable
# Node ID c058c6d1a857dac021d5377ce1bc96274cecae8e
# Parent  03350f5234a48daa3cdaf62f9c534c62b4bb46ed
manifest: fix possible SEGV caused by uninitialized lazymanifest fields

Before, uninitialized self->pydata would be passed to lazymanifest_dealloc()
on OOM, and Py_DECREF(self->pydata) would crash if we were unlucky.

It's still wrong to do malloc() thingy in tp_init because __init__() may be
called more than once [1], but I don't want to go a step further in stable
branch.

 [1]: https://docs.python.org/2/c-api/typeobj.html#c.PyTypeObject.tp_new
 "The tp_new function should ... do only as much further initialization as
  is absolutely necessary. Initialization that can safely be ignored or
  repeated should be placed in the tp_init handler."
Augie Fackler - June 15, 2018, 7:20 p.m.
> On Jun 15, 2018, at 09:54, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1529068618 -32400
> #      Fri Jun 15 22:16:58 2018 +0900
> # Branch stable
> # Node ID c058c6d1a857dac021d5377ce1bc96274cecae8e
> # Parent  03350f5234a48daa3cdaf62f9c534c62b4bb46ed
> manifest: fix possible SEGV caused by uninitialized lazymanifest fields

Queued for stable, thanks

Patch

diff --git a/mercurial/cext/manifest.c b/mercurial/cext/manifest.c
--- a/mercurial/cext/manifest.c
+++ b/mercurial/cext/manifest.c
@@ -135,12 +135,22 @@  static int find_lines(lazymanifest *self
 	return 0;
 }
 
+static void lazymanifest_init_early(lazymanifest *self)
+{
+	self->pydata = NULL;
+	self->lines = NULL;
+	self->numlines = 0;
+	self->maxlines = 0;
+}
+
 static int lazymanifest_init(lazymanifest *self, PyObject *args)
 {
 	char *data;
 	Py_ssize_t len;
 	int err, ret;
 	PyObject *pydata;
+
+	lazymanifest_init_early(self);
 	if (!PyArg_ParseTuple(args, "S", &pydata)) {
 		return -1;
 	}
@@ -668,6 +678,7 @@  static lazymanifest *lazymanifest_copy(l
 	if (!copy) {
 		goto nomem;
 	}
+	lazymanifest_init_early(copy);
 	copy->numlines = self->numlines;
 	copy->livelines = self->livelines;
 	copy->dirty = false;
@@ -705,6 +716,7 @@  static lazymanifest *lazymanifest_filter
 	if (!copy) {
 		goto nomem;
 	}
+	lazymanifest_init_early(copy);
 	copy->dirty = true;
 	copy->lines = malloc(self->maxlines * sizeof(line));
 	if (!copy->lines) {