oss-sec mailing list archives
Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling
From: Solar Designer <solar () openwall com>
Date: Mon, 23 Jan 2012 00:21:59 +0400
On Sun, Jan 22, 2012 at 09:52:27PM +0400, Solar Designer wrote:
On Wed, Jan 18, 2012 at 10:25:55AM +0800, Eugene Teo wrote:This changes it to do the permission checks at open time, and instead of tracking the process, it tracks the VM at the time of the open. That simplifies the code a lot, but does mean that if you hold the file descriptor open over an execve(), you'll continue to read from the _old_ VM.I see it in the revised code, but I don't get it. What does "the old VM" mean after an execve()? The code stores the mm pointer in file->private_data, but is this stored pointer even valid after an execve()? (The code blindly assumes so, only checking for non-NULL.)
OK, here's my current understanding: the old VM is preserved (refcounted) precisely because someone holds /proc/<pid>/mem open. The new mem_open() calls mm_access(), which calls get_task_mm(). So at the time of mem_read() / mem_write() the pointer is valid even if the process passed an execve(). However, I think this opens up a new security problem (albeit a relatively minor one): RLIMIT_NPROC * RLIMIT_AS bypass. Previously, a user with RLIMIT_NPROC set was sort of limited to consuming this much memory (plus shm and plus various in-kernel data structures related to the user's processes). Now the user's memory consumption via processes' address space is not limited by RLIMIT_NPROC anymore. Am I missing something? If not, I think we need to patch that. Maybe have RLIMIT_NPROC apply even to such "zombie VMs" (confusing and tricky). Maybe re-consider the entire approach to fixing the original issue addressed with commit e268337dfe26dfc7efd422a804dbb27977a3cccc. That is, revert this commit and fix the issue differently (likely by adding full privilege checks at time of read and write - in addition to re-introducing the self_exec_id checks, which are also needed even along with full checks of the caller's privileges). Alexander
Current thread:
- CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 17)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Kurt Seifried (Jan 17)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 17)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Kees Cook (Jan 18)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 18)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 20)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 23)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 18)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Kurt Seifried (Jan 17)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Solar Designer (Jan 22)
- Message not available
- Message not available
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Jason A. Donenfeld (Jan 22)
- Message not available
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Solar Designer (Jan 22)
- Re: CVE request: kernel: proc: clean up and fix /proc/<pid>/mem handling Eugene Teo (Jan 22)