On Sat, 14 Feb 2004, Alan Cox wrote:

> On Gwe, 2004-02-13 at 11:31, Thomas Biege wrote:
> > Hi,
> > one of our developers mentioned that depth->n can be negative.
> > 
> > I didn't checked the whole code but even if depth->n is unsigned,
> > count is signed and can be negative by using a depth->n > INT_MAX.
> > 
> > Is this a real problem or do we just hunt ghosts here?
> 
> I can't see how to exploit it offhand but its certainly soemthing that
> wants a better fix rolling into the next updates. I'm embarrassed I
> missed something so obvious

I attached my enhanced (hot-) fix patches. The same bugs appear in the 
drm4/ subdirectory too.

Alan, were is the upper limit of 4096 for 'count' from?

Bye,
     Thomas
-- 
  Thomas Biege <[EMAIL PROTECTED]>, SUSE LINUX AG, Security Support & Auditing
--
# If you have the "driftnet" program installed, webcollage can display a
# collage of images sniffed off your local ethernet, instead of pulled out
# of search engines: in that way, your screensaver can display the images
# that your co-workers are downloading!
                                          -- xscreensaver source-code
--- r128_state.c.bak    2004-02-16 15:39:53.000000000 +0100
+++ r128_state.c        2004-02-16 17:22:35.000000000 +0100
@@ -23,8 +23,22 @@
  * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * RED HAT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * THIS SOFTWARE IS NOT INTENDED FOR USE IN SAFETY CRITICAL SYSTEMS
+ *
  * Authors:
  *    Gareth Hughes <[EMAIL PROTECTED]>
+ *
+ * Memory allocation size checks added 14/01/2003, Alan Cox <[EMAIL PROTECTED]>
+ * Memory allocation size checks enhanced 16/02/2004, Thomas Biege <[EMAIL PROTECTED]>
+ *
  */
 
 #include "r128.h"
@@ -902,6 +916,9 @@ static int r128_cce_dispatch_write_span(
        DRM_DEBUG( "%s\n", __FUNCTION__ );
 
        count = depth->n;
+
+       if( count > 4096 || count <= 0)
+               return -EMSGSIZE;
        if ( copy_from_user( &x, depth->x, sizeof(x) ) ) {
                return -EFAULT;
        }
@@ -996,6 +1013,9 @@ static int r128_cce_dispatch_write_pixel
 
        count = depth->n;
 
+       if( count > 4096 || count <= 0)
+               return -EMSGSIZE;
+
        x = kmalloc( count * sizeof(*x), GFP_KERNEL );
        if ( x == NULL ) {
                return -ENOMEM;
@@ -1110,6 +1130,9 @@ static int r128_cce_dispatch_read_span( 
        DRM_DEBUG( "%s\n", __FUNCTION__ );
 
        count = depth->n;
+
+       if ( count > 4096 || count <= 0)
+               return -EMSGSIZE;
        if ( copy_from_user( &x, depth->x, sizeof(x) ) ) {
                return -EFAULT;
        }
@@ -1155,7 +1178,10 @@ static int r128_cce_dispatch_read_pixels
        if ( count > dev_priv->depth_pitch ) {
                count = dev_priv->depth_pitch;
        }
+       if( count * sizeof(int) <= 0 || count * sizeof(*x) <= 0 || count * sizeof(*y) 
<= 0)
+               return -EMSGSIZE;
 
+       
        x = kmalloc( count * sizeof(*x), GFP_KERNEL );
        if ( x == NULL ) {
                return -ENOMEM;
--- r128_state.c.bak    2004-02-16 15:40:14.000000000 +0100
+++ r128_state.c        2004-02-16 17:38:03.000000000 +0100
@@ -26,6 +26,8 @@
  * Authors:
  *    Gareth Hughes <[EMAIL PROTECTED]>
  *
+ * Memory allocation size checks added 16/02/2004, Thomas Biege <[EMAIL PROTECTED]>
+ *
  */
 
 #define __NO_VERSION__
@@ -938,6 +940,9 @@ static int r128_cce_dispatch_write_span(
        }
 
        count = depth->n;
+
+       if( count > 4096 || count <= 0)
+               return -EMSGSIZE;
        if ( copy_from_user( &x, depth->x, sizeof(x) ) ) {
                return -EFAULT;
        }
@@ -1047,6 +1052,9 @@ static int r128_cce_dispatch_write_pixel
        }
 
        count = depth->n;
+       if( count > 4096 || count <= 0 || count * sizeof(*x) <= 0 ||
+           count * sizeof(*y) <= 0)
+               return -EMSGSIZE;       
 
        x = kmalloc( count * sizeof(*x), 0 );
        if ( x == NULL ) {
@@ -1178,6 +1186,9 @@ static int r128_cce_dispatch_read_span( 
        }
 
        count = depth->n;
+
+       if ( count > 4096 || count <= 0)
+               return -EMSGSIZE;
        if ( copy_from_user( &x, depth->x, sizeof(x) ) ) {
                return -EFAULT;
        }
@@ -1238,6 +1249,8 @@ static int r128_cce_dispatch_read_pixels
        if ( count > dev_priv->depth_pitch ) {
                count = dev_priv->depth_pitch;
        }
+       if( count * sizeof(int) <= 0 || count * sizeof(*x) <= 0 || count * sizeof(*y) 
<= 0)
+               return -EMSGSIZE;
 
        x = kmalloc( count * sizeof(*x), 0 );
        if ( x == NULL ) {

Reply via email to