Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96933
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.68 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <MWHPR02MB247723E856E0DAF7639F0E26BFBF0@MWHPR02MB2477.namprd02.prod.outlook.com>
References: <MWHPR02MB247723E856E0DAF7639F0E26BFBF0@MWHPR02MB2477.namprd02.prod.outlook.com>
Date: Wed, 16 Nov 2016 18:03:50 +0100
Message-ID: <CAF+90c88UrJdVePRoqZs6ZbYSGt7uF5CN2TwT_c8WQru_392Jw@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Zeev Suraski <zeev@zend.com>, 
	"php-dev@coydogsoftware.net" <php-dev@coydogsoftware.net>, Julien Pauli <julienpauli@gmail.com>, 
	"rasmus@lerdorf.com" <rasmus@lerdorf.com>, "Anatol Belski (ab@php.net)" <ab@php.net>, Nikita Popov <nikic@php.net>, 
	Joe Watkins <pthreads@pthreads.org>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a114423829843a105416e0e0a
Subject: Re: [PATCH] opcache bug #69090, prepend user identifier to keys
From: nikita.ppv@gmail.com (Nikita Popov)

--001a114423829843a105416e0e0a
Content-Type: text/plain; charset=UTF-8

On Tue, Nov 15, 2016 at 6:32 PM, Dmitry Stogov <dmitry@zend.com> wrote:

> On Nov 15, 2016 18:50, Nikita Popov <nikita.ppv@gmail.com> wrote:
> >
> > On Tue, Nov 15, 2016 at 4:19 PM, Dmitry Stogov <dmitry@zend.com> wrote:
> >>
> >> New patch, attached to bug report, should fix both problems.
> >>
> >> I'm going to commit it tomorrow, if no objections.
> >>
> >>
> >> Thanks. Dmitry.
> >
> > For the new validate_root patch, wouldn't we still end up with inode
> collisions caused by the hash function? It looks like for inodes > 2^16
> collisions should be "common".
> >
> > Nikita
>
>
> It's not a problem to add inode check, but I think we don't need it.
>
>
> Let S1 and S2 strings, R1 and R2 root_hashes and F a hash function.
>
> You propose the following comparison for collision checks
>
>
> F(S1) ^ R1 == F(S2) ^ R2 && R1 == R2 && S1 == S2
>
>
> R1 == R2 seems useless. The existing condition:
>
>
> F(S1) ^ R1 == F(S2) ^ R2 && S1 == S2
>
>
> fails if S1 != S2 independently on R1 and R2.
>
> If S1 and S2 are the same, than F(S1) equal to F(S2) and consequently, to
> satisfy the whole condition, R1 should be equal to R2
>
>
> Am I wrong?
>
To clarify: My concern is not with the way the hash is compared, but that
the root hash itself could have collisions. That is, you have two root
inodes I1 and I2 and two root hashes R1 = H(I1), R2 = H(I2). In that case
it may be that R1 == R2, but I1 != I2. At least that's the way I understood
the patch -- it doesn't looks like there is anything explicitly preventing
this.

Nikita

--001a114423829843a105416e0e0a--