Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96825
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain coydogsoftware.net designates 162.246.56.219 as permitted sender)
Date: Thu, 10 Nov 2016 17:20:51 -0600
To: Dmitry Stogov <dmitry@zend.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>,
	"rasmus@lerdorf.com" <rasmus@lerdorf.com>,
	Zeev Suraski <zeev@zend.com>,
	"Anatol Belski (ab@php.net)" <ab@php.net>
Message-ID: <20161110232051.GA3031@sliver.coydogsoftware.net>
References: <c807ed0e31518329dd1c1c04ffea8bee@coydogsoftware.net>
 <MWHPR02MB2477D7E9020C36136D13F10BBFB80@MWHPR02MB2477.namprd02.prod.outlook.com>
 <20161110141019.GA5119@sliver.coydogsoftware.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161110141019.GA5119@sliver.coydogsoftware.net>
User-Agent: Mutt/1.5.20 (2009-12-10)
Subject: Re: [PATCH] opcache bug #69090, prepend user identifier to keys
From: php-dev@coydogsoftware.net

Hello Dmitry,

A quick question after considering your comments and doing a bit more
homework on extension lifetime:

On Thu, Nov 10, 2016 at 08:10:19AM -0600, php-dev@coydogsoftware.net wrote:
> 
> On Thu, Nov 10, 2016 at 08:51:58AM +0000, Dmitry Stogov wrote:
> > 
> > The patch introduces syscall in the hot function (this may be
> > optimized).
> 
> Agreed. That isn't ideal. But the geteuid() call shouldn't be done
> during opcache initialization when the SHM object is initialized,
> because EUID might change afterwards. I didn't want to get EUID too
> early so I erred on the side of caution, getting it at the last possible
> moment. This is slower but safer because it prevents trivial cross-user
> cache access from PHP userland. I'm open to suggestion if there's a more
> "local" initialization function outside of key generation, which is
> guaranteed to run after EUID changes in both FPM user pool, and
> mod_ruid2/mod_php.

Looks like OPCache currently does not implement a request_startup_func for
per-request initialization. I'm assuming this was avoided for
performance reasons and due to no perceived need until now for
per-request initialization. This seems like it would be the proper point
in extension lifetime to call geteuid() and save results in the
zend_accel_globals struct to use during key construction. Correct? Any
objection to my implementing this?

I'm also thinking the stat() for device+inode should be done in opcache
code at file compilation time; correct?

Would this be a step in the right direction? Apologies if these are
basic questions; I'm new to PHP extensions but strongly motivated to fix
OPCache keys quickly. I may have time for a second pass at this over the
weekend.

-php-dev at coydogsoftware dot net