[PATCH 06 of 10 V2] revlog: add a native implementation of issnapshot

Yuya Nishihara yuya at tcha.org
Fri Dec 21 23:07:25 EST 2018


On Fri, 21 Dec 2018 12:47:09 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1545040633 -3600
> #      Mon Dec 17 10:57:13 2018 +0100
> # Node ID d15f3f46ca1e52f06d5b8f3f77c5e2e02912aaa9
> # Parent  4067f496c37ec6d10860504186917bb6be83db46
> # EXP-Topic sparse-revlog
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r d15f3f46ca1e
> revlog: add a native implementation of issnapshot

> +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);

Here base isn't guaranteed to be a valid revision, and passing it in to
index_issnapshotrev() will allow arbitrary memory read.

> +	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);

Maybe you missed the comment in the previous series, but this function will
never stop if base goes e.g. upwards at some point, and SEGV. Appears that
gcc can turn the recursion into loop in this case, but still there's a risk
of infinite loop.


More information about the Mercurial-devel mailing list