From @Leont

This is a bug report for perl from fawaka@​gmail.com,
generated with the help of perlbug 1.39 running under perl 5.14.1.

Currently, the real uid, the effective uid, the real gid and the effective gid are cached by perl. This can result in erroneous results if something other that core changes any of these process properties. This issue is similar to #85520.

Flags​:
  category=core
  severity=low

Site configuration information for perl 5.14.1​:

Configured by leon at Tue Jun 28 15​:59​:52 CEST 2011.

Summary of my perl5 (revision 5 version 14 subversion 1) configuration​:
 
  Platform​:
  osname=linux, osvers=2.6.38-8-generic, archname=x86_64-linux
  uname='linux leon-laptop 2.6.38-8-generic #42-ubuntu smp mon apr 11 03​:31​:24 utc 2011 x86_64 x86_64 x86_64 gnulinux '
  config_args='-de -Dprefix=/home/leon/perl5/perlbrew/perls/perl-5.14.1'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.6.1 20110409 (prerelease)', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /usr/lib/x86_64-linux-gnu /lib64 /usr/lib64
  libs=-lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.13'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 

@​INC for perl 5.14.1​:
  /home/leon/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1/x86_64-linux
  /home/leon/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1
  /home/leon/perl5/perlbrew/perls/perl-5.14.1/lib/5.14.1/x86_64-linux
  /home/leon/perl5/perlbrew/perls/perl-5.14.1/lib/5.14.1
  .

Environment for perl 5.14.1​:
  HOME=/home/leon
  LANG=en_US.utf8
  LANGUAGE=en_US​:en
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/leon/perl5/perlbrew/bin​:/home/leon/perl5/perlbrew/perls/perl-5.14.1/bin​:/home/leon/bin​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/bin​:/usr/games
  PERLBREW_HOME=/home/leon/.perlbrew
  PERLBREW_PATH=/home/leon/perl5/perlbrew/bin​:/home/leon/perl5/perlbrew/perls/perl-5.14.1/bin
  PERLBREW_PERL=perl-5.14.1
  PERLBREW_ROOT=/home/leon/perl5/perlbrew
  PERLBREW_VERSION=0.25
  PERL_BADLANG (unset)
  SHELL=/bin/bash

From @jkeenan

On Wed Aug 03 08​:10​:28 2011, LeonT wrote​:

Currently, the real uid, the effective uid, the real gid and the
effective gid are cached by perl. This can result in erroneous
results if something other that core changes any of these process
properties. This issue is similar to #85520.

Is there any way we could get a small program that demonstrates this bug?

Thank you very much.
Jim Keenan

The RT System itself - Status changed from 'new' to 'open'

From @Leont

On Tue Dec 06 04​:08​:51 2011, jkeenan wrote​:

Is there any way we could get a small program that demonstrates this bug?

See attachment. (It requires a working syscall.ph file though)

Leon

From @Leont

uid.pl

From @Leont

On Sun, Jan 15, 2012 at 9​:57 PM, Leon Timmermans via RT
<perlbug-followup@​perl.org> wrote​:

See attachment. (It requires a working syscall.ph file though)

D'oh!

I was sleeping, and showed the same issue for the PID instead of the
UID. Here's the correct program.

Leon

From @Leont

uid.pl

From @Leont

On Sun, Jan 15, 2012 at 10​:03 PM, Leon Timmermans <fawaka@​gmail.com> wrote​:

D'oh!

I was sleeping, and showed the same issue for the PID instead of the
UID. Here's the correct program.

Third attempt. I really should stop coding for tonight.

Leon

From @Leont

uid.pl

From @cpansprout

On Sun Jan 15 13​:08​:17 2012, LeonT wrote​:

On Sun, Jan 15, 2012 at 10​:03 PM, Leon Timmermans <fawaka@​gmail.com>
wrote​:

D'oh!

I was sleeping, and showed the same issue for the PID instead of the
UID. Here's the correct program.

Third attempt. I really should stop coding for tonight.

But it’s early afternoon! :-)

--

Father Chrysostomos

From @Leont

On Sun, Jan 15, 2012 at 10​:07 PM, Leon Timmermans <fawaka@​gmail.com> wrote​:

Third attempt. I really should stop coding for tonight.

Fix attached. I couldn't remove the cache entirely, as it's also used
by the ID swapping, but now every read of $&lt;, $&gt;, $( or $) always
returns the correct result.

Leon

From @Leont

From 61e31001d5a99be3815b06315075e9a4231fadb3 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <fawaka@gmail.com>
Date: Sat, 11 Feb 2012 20:41:26 +0100
Subject: [PATCH] Don't cache gete?[ug]id

This could lead to subtle issues when the UID setting bypasses perl
---
 mg.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mg.c b/mg.c
index 14e9705..3762cd5 100644
--- a/mg.c
+++ b/mg.c
@@ -1109,16 +1109,16 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
 	SvNOK_on(sv);	/* what a wonderful hack! */
 	break;
     case '<':
-	sv_setiv(sv, (IV)PL_uid);
+	sv_setiv(sv, (IV)PerlProc_getuid());
 	break;
     case '>':
-	sv_setiv(sv, (IV)PL_euid);
+	sv_setiv(sv, (IV)PerlProc_geteuid());
 	break;
     case '(':
-	sv_setiv(sv, (IV)PL_gid);
+	sv_setiv(sv, (IV)PerlProc_getgid());
 	goto add_groups;
     case ')':
-	sv_setiv(sv, (IV)PL_egid);
+	sv_setiv(sv, (IV)PerlProc_getegid());
       add_groups:
 #ifdef HAS_GETGROUPS
 	{
-- 
1.7.5.4

From @avar

On Sat, Feb 11, 2012 at 20​:45, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Sun, Jan 15, 2012 at 10​:07 PM, Leon Timmermans <fawaka@​gmail.com> wrote​:

Third attempt. I really should stop coding for tonight.

Fix attached. I couldn't remove the cache entirely, as it's also used
by the ID swapping, but now every read of $&lt;, $&gt;, $( or $) always
returns the correct result.

I very much like where this patch is going, but IMO it needs some
improvement​:

* You've changed it so that we now return the current and correct
  values when getting $&lt;, $&gt;, $( and $), but there's still a lot of
  places in the perl core where we're looking at PL_uid and pals,
  these should be changed to call PerlProc_getuid et al instead.

* Since the id swapping is only used by "PL_delaymagic &= ~DM_RUID;"
  and pp_sassign (and variants) we should just add new a new
  PL_delaymagic_uid variable that's only used by the set() magic and
  sassign().

  This variable should not be made public. Thus programs on the CPAN
  that expect to assign to PL_uid would fail and would need to be
  updated, but they wouldn't contain a logic error anymore (by
  expecting the core to read from PL_uid).

* Code like the code in this ifdef in POSIX.xs can just go away​:

  SysRet
  setuid(uid)
  Uid_t uid
  CLEANUP​:
  #ifndef WIN32
  if (RETVAL >= 0) {
  PL_uid = getuid();
  PL_euid = geteuid();
  }
  #endif

* It would also be informative to check how much of the CPAN is
  relying on PL_uid, but I don't think that should block this going
  in.

From @Leont

On Sat, Feb 11, 2012 at 9​:42 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

I very much like where this patch is going, but IMO it needs some
improvement​:

 * You've changed it so that we now return the current and correct
  values when getting $&lt;, $&gt;, $( and $), but there's still a lot of
  places in the perl core where we're looking at PL_uid and pals,
  these should be changed to call PerlProc_getuid et al instead.

Yeah, that'd be a good idea. Though doing it properly may invite some
refactoring

 * Since the id swapping is only used by "PL_delaymagic &= ~DM_RUID;"
  and pp_sassign (and variants) we should just add new a new
  PL_delaymagic_uid variable that's only used by the set() magic and
  sassign().

  This variable should not be made public. Thus programs on the CPAN
  that expect to assign to PL_uid would fail and would need to be
  updated, but they wouldn't contain a logic error anymore (by
  expecting the core to read from PL_uid).

 * Code like the code in this ifdef in POSIX.xs can just go away​:

   SysRet
   setuid(uid)
           Uid_t           uid
       CLEANUP​:
   #ifndef WIN32
           if (RETVAL >= 0) {
               PL_uid  = getuid();
               PL_euid = geteuid();
           }
   #endif

 * It would also be informative to check how much of the CPAN is
  relying on PL_uid, but I don't think that should block this going
  in.

I took my approach because it's easiest. Existing code would
effectively become a no-op. In principle it's a good idea, but given
this may break stuff on CPAN, depends on #1 and we're approaching the
user-visible features deadline fast I'd rather postpone it until 5.17.

Leon

From @avar

On Sat, Feb 11, 2012 at 22​:53, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Sat, Feb 11, 2012 at 9​:42 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

I very much like where this patch is going, but IMO it needs some
improvement​:

 * You've changed it so that we now return the current and correct
  values when getting $&lt;, $&gt;, $( and $), but there's still a lot of
  places in the perl core where we're looking at PL_uid and pals,
  these should be changed to call PerlProc_getuid et al instead.

Yeah, that'd be a good idea. Though doing it properly may invite some
refactoring

 * Since the id swapping is only used by "PL_delaymagic &= ~DM_RUID;"
  and pp_sassign (and variants) we should just add new a new
  PL_delaymagic_uid variable that's only used by the set() magic and
  sassign().

  This variable should not be made public. Thus programs on the CPAN
  that expect to assign to PL_uid would fail and would need to be
  updated, but they wouldn't contain a logic error anymore (by
  expecting the core to read from PL_uid).

 * Code like the code in this ifdef in POSIX.xs can just go away​:

   SysRet
   setuid(uid)
           Uid_t           uid
       CLEANUP​:
   #ifndef WIN32
           if (RETVAL >= 0) {
               PL_uid  = getuid();
               PL_euid = geteuid();
           }
   #endif

 * It would also be informative to check how much of the CPAN is
  relying on PL_uid, but I don't think that should block this going
  in.

I took my approach because it's easiest. Existing code would
effectively become a no-op. In principle it's a good idea, but given
this may break stuff on CPAN, depends on #1 and we're approaching the
user-visible features deadline fast I'd rather postpone it until 5.17.

I've implemented my proposal and pushed it to
avar/remove-get-uid-caching
(http​://perl5.git.perl.org/perl.git/commitdiff/5123dd783c7) here's the
commit message for reference​:

  Remove gete?[ug]id caching

  Currently we cache the UID/GID and effective UID/GID similarly to how
  we used to cache getpid() before v5.14.0-251-g0e21945. Remove this
  magical behavior in favor of always calling getpid(), getgid()
  etc. This resolves RT #96208.

  A minimal testcase for this is the following by Leon Timmermans
  attached to RT #96208​:

  eval { require 'syscall.ph'; 1 } or eval { require
'sys/syscall.ph'; 1 } or die $@​;

  if (syscall(&SYS_setuid, $ARGV[0] + 0 || 1000) &gt;= 0 or die "$!") {
  printf "\$< = %d, getuid = %d\n", $<, syscall(&SYS_getuid);
  }

  I.e. if we call the sete?[ug]id() functions unbeknownst to perl the
  $&lt;, $&gt;, $( and $) variables won't be updated. This results in the same
  sort of issues we had with $$ before v5.14.0-251-g0e21945, and
  getppid() before my "Further eliminate POSIX-emulation under
  LinuxThreads" patch.

  I'm completely eliminating the PL_egid, PL_euid, PL_gid and PL_uid
  variables as part of this patch, this will break some CPAN modules,
  but it'll be really easy before the v5.16.0 final to reinstate
  them. I'd like to remove them to see what breaks, and how easy it is
  to fix it.

  The new PL_delaymagic_(egid|euid|gid|uid) variables I'm adding are
  only intended to be used internally in the interpreter to facilitate
  the delaymagic in sassign. There's probably some way not to export
  these to programs that embed perl, but I haven't found out how to do
  that.

  I don't *think* this has any bugs, but I haven't extensively tested
  it, and it seems there's no extensive tests for these variables in our
  test suite, this needs to be fixed before this patch goes into blead.

Review of this would be very appreciated, especially of the tricky
sections in mg.c and pp_hot.c. It still needs tests, but aside from
that, the perldelta entry and any small bugs in the patch I think it's
ready for blead.

From @Leont

On Sun, Feb 12, 2012 at 8​:07 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

   I don't *think* this has any bugs, but I haven't extensively tested
   it, and it seems there's no extensive tests for these variables in our
   test suite, this needs to be fixed before this patch goes into blead.

One annoying and unusual complication is that testing is only possible
for root. I don't expect a lot of automated testing.

Review of this would be very appreciated, especially of the tricky
sections in mg.c and pp_hot.c. It still needs tests, but aside from
that, the perldelta entry and any small bugs in the patch I think it's
ready for blead.

Looked ok on first read, but there may be trickiness indeed.

Leon

From @avar

On Sun Feb 12 11​:09​:03 2012, avarab@​gmail.com wrote​:

I've implemented my proposal and pushed it to
avar/remove-get-uid-caching
(http​://perl5.git.perl.org/perl.git/commitdiff/5123dd783c7) here's the
commit message for reference​:

I've pushed an altered version of this as 985213f, which fixes this
bug.

@avar - Status changed from 'open' to 'resolved'

From @avar

On Sun Feb 12 11​:09​:03 2012, avarab@​gmail.com wrote​:

I've implemented my proposal and pushed it to
avar/remove-get-uid-caching
(http​://perl5.git.perl.org/perl.git/commitdiff/5123dd783c7) here's the
commit message for reference​:

I've pushed an altered version of this as 985213f, which fixes this
bug.