Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1452744

Re: [PATCH] prctl: remove one-shot limitation for changing exe link

From Mateusz Guzik <mguzik@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] prctl: remove one-shot limitation for changing exe link
Date 2016-07-30 22:30 +0200
Message-ID <s0J3r-7ef-1@gated-at.bofh.it> (permalink)
References (2 earlier) <rYSXg-7Yy-9@gated-at.bofh.it> <rYTJD-8ug-15@gated-at.bofh.it> <rYUw1-B7-19@gated-at.bofh.it> <rZ64a-884-9@gated-at.bofh.it> <s0GyB-5wI-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sat, Jul 30, 2016 at 12:31:40PM -0500, Eric W. Biederman wrote:
> So what I am requesting is very simple.  That the checks in
> prctl_set_mm_exe_file be tightened up to more closely approach what
> execve requires.  Thus preserving the value of the /proc/[pid]/exe for
> the applications that want to use the exe link.
> 
> Once the checks in prctl_set_mm_exe_file are tightened up please feel
> free to remove the one shot test.
> 

This is more fishy.

First of all exe_file is used by the audit subsystem. So someone has to
ask audit people what is the significance (if any) of the field.

All exe_file users but one use get_mm_exe_file and handle NULL
gracefully.

Even with the current limit of changing the field once, the user can
cause a transient failure of get_mm_exe_file which can fail to increment
the refcount before it drops to 0.

This transient failure can be used to get a NULL value stored in
->exe_file during fork (in dup_mmap):
RCU_INIT_POINTER(mm->exe_file, get_mm_exe_file(oldmm));

The one place which is not using get_mm_exe_file to get to the pointer
is audit_exe_compare:
        rcu_read_lock();
        exe_file = rcu_dereference(tsk->mm->exe_file);
        ino = exe_file->f_inode->i_ino;
        dev = exe_file->f_inode->i_sb->s_dev;
        rcu_read_unlock();

This is buggy on 2 accounts:
1. exe_file can be NULL
2. rcu does not protect f_inode

The issue is made worse with allowing arbitrary number changes.

Modifying get_mm_exe_file to retry is trivial and in effect never return
NULL is trivial. With arbitrary number of changes allowed this may
require some cond_resched() or something.

For comments I cc'ed Richard Guy Briggs, who is both an audit person and
the author of audit_exe_compare.
-- 
Mateusz Guzik

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH] prctl: remove one-shot limitation for changing exe link Cyrill Gorcunov <gorcunov@gmail.com> - 2016-07-26 10:40 +0200
  Re: [PATCH] prctl: remove one-shot limitation for changing exe link ebiederm@xmission.com (Eric W. Biederman) - 2016-07-30 19:50 +0200
    Re: [PATCH] prctl: remove one-shot limitation for changing exe link Mateusz Guzik <mguzik@redhat.com> - 2016-07-30 22:30 +0200
      Re: [PATCH] prctl: remove one-shot limitation for changing exe link ebiederm@xmission.com (Eric W. Biederman) - 2016-07-31 21:00 +0200
    Re: [PATCH] prctl: remove one-shot limitation for changing exe link Cyrill Gorcunov <gorcunov@gmail.com> - 2016-08-01 00:50 +0200
      Re: [PATCH] prctl: remove one-shot limitation for changing exe link Andy Lutomirski <luto@amacapital.net> - 2016-08-01 01:00 +0200
    Re: [PATCH] prctl: remove one-shot limitation for changing exe link Cyrill Gorcunov <gorcunov@gmail.com> - 2016-08-01 11:30 +0200

csiph-web