This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] posix: Fix posix_spawnpi to not execute invalid binaries in non compat mode (BZ#23264)



On 07/06/2018 11:10, Adhemerval Zanella wrote:
> 
> 
> On 07/06/2018 09:44, Florian Weimer wrote:
>> On 06/06/2018 08:26 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 06/06/2018 15:20, Florian Weimer wrote:
>>>> On 06/06/2018 08:11 PM, Adhemerval Zanella wrote:
>>>>> Current posix_spawnp implementation wrongly tries to execute invalid
>>>>> binaries (for instance script without shebang) as a shell script in
>>>>> non compat mode.  It was a regression introduced by
>>>>> 9ff72da471a509a8c19791efe469f47fa6977410 when __spawni started to use
>>>>> __execvpe instead of __execve (glibc __execvpe try to execute ENOEXEC
>>>>> as shell script regardless).
>>>>>
>>>>> This patch fixes it by using an internal symbol (__execvpex) with the
>>>>> faulty semantic (since compat mode is handled by spawni.c itself).
>>>>
>>>> Why doesn't this need a new compatibility symbol, similar to the previously attempted change?
>>>
>>> Should we handle regressions in such way? My intention is to backport it
>>> to previous versions as well.
>>
>> Hmm, right, it's a regression.
>>
>> So this leads to the question why we need to keep the old compat behavior.  Maybe we can just alias the two symbol versions to a single implementation?  Is it really necessary to preserve the old script execution behavior?
> 
> I am not sure in fact and indeed old behaviour is a potential source of
> security issues (by invoking shell script in non executable binary).  
> I would prefer to just drop old behaviour, which would simplify the
> implementation a bit, but it would break some compat assumptions. 
> Do you see any gain in continue support this behaviour on posix_spawn
> (we can discuss it if execvpe also is worth as well)? 
> 

I think we can evaluate this change as follow up patch, I would like to
at least fix this regression for new release.  Do you have any objections
to the patch?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]