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 ) {