Patchwork [5,of,8] revlog: add a native implementation of issnapshot

login
register
mail settings
Submitter Boris Feld
Date Dec. 17, 2018, noon
Message ID <72f9ed348875eaa260f4.1545048047@localhost.localdomain>
Download mbox | patch
Permalink /patch/37221/
State Accepted
Headers show

Comments

Boris Feld - Dec. 17, 2018, noon
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1545040633 -3600
#      Mon Dec 17 10:57:13 2018 +0100
# Node ID 72f9ed348875eaa260f4536885c3120ae536f373
# Parent  9fe9cc49f235311269fd957c49898396ed7bdfc0
# EXP-Topic sparse-revlog-corner-cases
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 72f9ed348875
revlog: add a native implementation of issnapshot

This will be used in the next changesets
Yuya Nishihara - Dec. 17, 2018, 12:59 p.m.
On Mon, 17 Dec 2018 12:00:47 +0000, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1545040633 -3600
> #      Mon Dec 17 10:57:13 2018 +0100
> # Node ID 72f9ed348875eaa260f4536885c3120ae536f373
> # Parent  9fe9cc49f235311269fd957c49898396ed7bdfc0
> # EXP-Topic sparse-revlog-corner-cases
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 72f9ed348875
> revlog: add a native implementation of issnapshot

How well this and the subsequent C functions are covered by tests? Can you
first add more rock-solid tests for the previous series?

> +static int index_issnapshotrev(indexObject *self, Py_ssize_t rev)
> +{
> +	int ps[2];
> +	Py_ssize_t base;
> +	if (rev == -1) {
> +		return 1;
> +	}
> +	base = (Py_ssize_t)index_baserev(self, rev);
> +	if (base == rev) {
> +		base = -1;
> +	}
> +	if (base == -2) {
> +		assert(PyErr_Occurred());
> +		return -1;
> +	}
> +	if (base == -1) {
> +		return 1;
> +	}
> +	if (index_get_parents(self, rev, ps, (int)rev) < 0) {
> +		assert(PyErr_Occurred());
> +		return -1;
> +	};
> +	if (base == ps[0] || base == ps[1]) {
> +		return 0;
> +	}
> +	return index_issnapshotrev(self, base);

C stack boundary isn't checked. I suspect this could lead to stack overflow
if malicious revlog is fed.

I didn't check if there are other types of security issues.

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -986,6 +986,34 @@  static inline int index_baserev(indexObj
 	}
 }
 
+static int index_issnapshotrev(indexObject *self, Py_ssize_t rev)
+{
+	int ps[2];
+	Py_ssize_t base;
+	if (rev == -1) {
+		return 1;
+	}
+	base = (Py_ssize_t)index_baserev(self, rev);
+	if (base == rev) {
+		base = -1;
+	}
+	if (base == -2) {
+		assert(PyErr_Occurred());
+		return -1;
+	}
+	if (base == -1) {
+		return 1;
+	}
+	if (index_get_parents(self, rev, ps, (int)rev) < 0) {
+		assert(PyErr_Occurred());
+		return -1;
+	};
+	if (base == ps[0] || base == ps[1]) {
+		return 0;
+	}
+	return index_issnapshotrev(self, base);
+}
+
 static PyObject *index_deltachain(indexObject *self, PyObject *args)
 {
 	int rev, generaldelta;