Front page | perl.perl5.porters |
Postings from February 2012
Re: [perl #96208] UIDs and GIDs should not be cached
Thread Previous
|
Thread Next
From:
Ævar Arnfjörð Bjarmason
Date:
February 12, 2012 11:08
Subject:
Re: [perl #96208] UIDs and GIDs should not be cached
Message ID:
CACBZZX7WVkm1cg6cujJ7H-9Dy9_v-g0JCgXSs7nXOX+hnbSV3w@mail.gmail.com
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 $<, $>, $( 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) >= 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
$<, $>, $( 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.
Thread Previous
|
Thread Next