Bugzilla Bug 80107

Georgi Guninski has discovered three important sign comparison problems in v2.6 
kernels. 

The collection of vulnerabilities has led Linus to add appropriate range
checking at higher VFS levels (ie sys_read{v}/sys_write{v}):

        ChangeSet@1.1966.1.68, 2005-01-25 20:29:47-08:00,
torvalds@ppc970.osdl.org
          Add 'f_maxcount' to allow filesystems to set a per-file maximum IO
size.

        ChangeSet@1.1966.1.67, 2005-01-25 15:00:33-08:00,
torvalds@ppc970.osdl.org
          Rename "locks_verify_area()" to "rw_verify_area()" and clean up the
          arguments.

With such range checking in place those bugs will be protected (can't be 
be triggered) by the higher levels - they should be fixed nevertheless.

The following fixes are going into Linus's tree Wednesday, so this date
(February 2) can 
be though of as the release date. 

There are for sure more sign handling bugs lying in drivers/fs'es. 

Kudos again to Georgi for his bright auditing! 

1) drivers/char/n_tty.c::copy_from_read_buf()

"nr" is user passed parameter, so if its value is higher than MAX_INT, the
signed "min" will 
happily return "nr", causing unexpected results.

Note that "n" is used afterwards as lenght parameter of copy_to_user:

retval = copy_to_user(*b, &tty->read_buf[tty->read_tail], n);

This fixes it as suggested by Linus:

--- drivers/char/n_tty.c.orig   2005-01-20 13:33:30.000000000 -0200
+++ drivers/char/n_tty.c        2005-01-30 13:56:05.038069640 -0200
@@ -1143,13 +1143,13 @@

 {
        int retval;
-       ssize_t n;
+       size_t n;
        unsigned long flags;

        retval = 0;
        spin_lock_irqsave(&tty->read_lock, flags);
        n = min(tty->read_cnt, N_TTY_BUF_SIZE - tty->read_tail);
-       n = min((ssize_t)*nr, n);
+       n = min(*nr, n);
        spin_unlock_irqrestore(&tty->read_lock, flags);
        if (n) {
                mb();

2) fs/proc/generic.c::proc_file_read() 

"nbytes" is user passed parameter, so if its value is higher than MAX_INT, the
signed
"min" comparison returns "nbytes".

Several proc handling functions do not expect huge values. get_locks_status()
for example
(handler for /proc/locks) can be exploited to read kernel memory.

--- fs/proc/generic.c.orig      2005-01-20 12:39:42.000000000 -0200
+++ fs/proc/generic.c   2005-01-30 13:58:00.420528840 -0200
@@ -60,7 +60,7 @@
                return -ENOMEM;

        while ((nbytes > 0) && !eof) {
-               count = min_t(ssize_t, PROC_BLOCK_SIZE, nbytes);
+               count = min_t(size_t, PROC_BLOCK_SIZE, nbytes);

                start = NULL;
                if (dp->get_info) {

3) reiserfs: 

NOTE: The fix is incomplete, reiserfs_copy_from_user_to_file_region() also
suffers
from similar issue but is not fixed in the following patch. The reiserfs people 
should provide a more appropriate fix.

reiserfs_file_write() casts its (size_t) count parameter to int, which can
become
a problem on 64-bit architectures because the following range check will fail
if for eg count is "0x1ffffffff":

       if ( unlikely((ssize_t) count < 0)) 
              return -EINVAL;

Which will later be casted to "int" here:

        int write_bytes; /* amount of bytes to write during this iteration */

        if ( !num_pages ) { /* If we do not have enough space even for */
            res = -ENOSPC;  /* single page, return -ENOSPC */
            if ( pos > (inode->i_size & (inode->i_sb->s_blocksize-1)))
                break; // In case we are writing past the file end, break.
            // Otherwise we are possibly overwriting the file, so
            // let's set write size to be equal or less than blocksize.
            // This way we get it correctly for file holes.
            // But overwriting files on absolutelly full volumes would not
            // be very efficient. Well, people are not supposed to fill
            // 100% of disk space anyway.
            write_bytes = min_t(int, count, inode->i_sb->s_blocksize - (pos &
(inode->i_sb->s_blocksize - 1)));

Making "write_bytes" negative, which potentially causes unexpected results. 

The following patch is an attempt to fix this by changing the variables dealing 
with count and offset and the "min_t" comparisons to unsigned "size_t". 

--- file.c.orig 2005-01-25 16:01:13.000000000 -0200
+++ file.c      2005-01-26 13:28:11.819375848 -0200
@@ -588,7 +588,7 @@ error_exit:

 /* Unlock pages prepared by reiserfs_prepare_file_region_for_write */
 static void reiserfs_unprepare_pages(struct page **prepared_pages, /* list of
locked pages */
-                             int num_pages /* amount of pages */) {
+                             size_t num_pages /* amount of pages */) {
     int i; // loop counter

     for (i=0; i < num_pages ; i++) {
@@ -619,7 +619,7 @@ static int reiserfs_copy_from_user_to_fi
     int offset; // offset in page

     for ( i = 0, offset = (pos & (PAGE_CACHE_SIZE-1)); i < num_pages ;
i++,offset=0) {
-       int count = min_t(int,PAGE_CACHE_SIZE-offset,write_bytes); // How much
of bytes to write to this page
+       size_t count = min_t(size_t,PAGE_CACHE_SIZE-offset,write_bytes); // How
much of bytes to write to this page
        struct page *page=prepared_pages[i]; // Current page we process.

        fault_in_pages_readable( buf, count);
@@ -718,8 +718,8 @@ static int reiserfs_submit_file_region_f
                                struct reiserfs_transaction_handle *th,
                                struct inode *inode,
                                loff_t pos, /* Writing position offset */
-                               int num_pages, /* Number of pages to write */
-                               int write_bytes, /* number of bytes to write */
+                               size_t num_pages, /* Number of pages to write
*/
+                               size_t write_bytes, /* number of bytes to write
*/
                                struct page **prepared_pages /* list of pages
*/
                                )
 {
@@ -854,9 +854,9 @@ static int reiserfs_check_for_tail_and_c
 static int reiserfs_prepare_file_region_for_write(
                                struct inode *inode /* Inode of the file */,
                                loff_t pos, /* position in the file */
-                               int num_pages, /* number of pages to
+                               size_t num_pages, /* number of pages to
                                                  prepare */
-                               int write_bytes, /* Amount of bytes to be
+                               size_t write_bytes, /* Amount of bytes to be
                                                    overwritten from
                                                    @pos */
                                struct page **prepared_pages /* pointer to
array
@@ -1252,10 +1252,9 @@ static ssize_t reiserfs_file_write( stru
     while ( count > 0) {
        /* This is the main loop in which we running until some error occures
           or until we write all of the data. */
-       int num_pages;/* amount of pages we are going to write this iteration
*/
-       int write_bytes; /* amount of bytes to write during this iteration */
-       int blocks_to_allocate; /* how much blocks we need to allocate for
-                                  this iteration */
+       size_t num_pages;/* amount of pages we are going to write this
iteration */
+       size_t write_bytes; /* amount of bytes to write during this iteration
*/
+       size_t blocks_to_allocate; /* how much blocks we need to allocate for
this iteration */

         /*  (pos & (PAGE_CACHE_SIZE-1)) is an idiom for offset into a page of
pos*/
        num_pages = !!((pos+count) & (PAGE_CACHE_SIZE - 1)) + /* round up
partial
@@ -1269,7 +1268,7 @@ static ssize_t reiserfs_file_write( stru
            /* If we were asked to write more data than we want to or if there
               is not that much space, then we shorten amount of data to write
               for this iteration. */
-           num_pages = min_t(int, REISERFS_WRITE_PAGES_AT_A_TIME,
reiserfs_can_fit_pages(inode->i_sb));
+           num_pages = min_t(size_t, REISERFS_WRITE_PAGES_AT_A_TIME,
reiserfs_can_fit_pages(inode->i_sb));
            /* Also we should not forget to set size in bytes accordingly */
            write_bytes = (num_pages << PAGE_CACHE_SHIFT) - 
                            (pos & (PAGE_CACHE_SIZE-1));
@@ -1295,7 +1294,7 @@ static ssize_t reiserfs_file_write( stru
            // But overwriting files on absolutelly full volumes would not
            // be very efficient. Well, people are not supposed to fill
            // 100% of disk space anyway.
-           write_bytes = min_t(int, count, inode->i_sb->s_blocksize - (pos &
(inode->i_sb->s_blocksize - 1)));
+           write_bytes = min_t(size_t, count, inode->i_sb->s_blocksize - (pos
& (inode->i_sb->s_blocksize - 1)));
            num_pages = 1;
            // No blocks were claimed before, so do it now.
            reiserfs_claim_blocks_to_be_allocated(inode->i_sb, 1 <<
(PAGE_CACHE_SHIFT - inode->i_blkbits));

This is a dupe for bug 82141 and bug 81295, maybe left open until we can locate
a more official patch...

*** This bug has been marked as a duplicate of 82141 ***