Re: [PATCH v2] fs: Allow opening only regular files during execve().
From: Tetsuo Handa
Date: Tue Feb 19 2019 - 04:52:26 EST
Hello, Al.
What would you suggest if FMODE_EXEC is not the right choice?
On 2019/01/22 9:51, Kees Cook wrote:
> On Tue, Jan 22, 2019 at 10:18 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>> We are *NOT* going to use current->in_execve to propagate that information.
>> Come up with a cleaner solution, if you care, but this one is a non-starter.
>> Too ugly to live. Sorry.
>
> What would you suggest for a cleaner indication of being in an execve?
>
On 2019/02/12 14:26, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
>> Andrew Morton wrote:
>>>> --- a/fs/open.c
>>>> +++ b/fs/open.c
>>>> @@ -733,6 +733,12 @@ static int do_dentry_open(struct file *f,
>>>> return 0;
>>>> }
>>>>
>>>> + /* Any file opened for execve()/uselib() has to be a regular file. */
>>>> + if (unlikely(f->f_flags & FMODE_EXEC && !S_ISREG(inode->i_mode))) {
>>>> + error = -EACCES;
>>>> + goto cleanup_file;
>>>> + }
>>>> +
>>>> if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
>>>> error = get_write_access(inode);
>>>> if (unlikely(error))
>>>
>>> This change sounds legitimate for various other reasons, but it's a
>>> concern that this locking error occurred in the first place. There's a
>>> problem somewhere (probably the pipe code) which may bite us in other
>>> situations, even with this workaround in place.
>>>
>>>
>>
>> This error seems to be introduced in 2.6.39-rc1 by commit a9712bc12c40c172
>> ("deal with races in /proc/*/{syscall,stack,personality}"). Thus, I don't
>> think that this patch will bite us in other situations.
>> Al, how do you want to handle this?
>>
>
> According to bisection, commit 8924feff66f35fe2 ("splice: lift pipe_lock out of splice_to_pipe()")
> added in 4.9-rc1 is the trigger of this deadlock. Thus, the fix needs to go to 4.9+.