lkml.org 
[lkml]   [2005]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [Coverity] Untrusted user data in kernel
    : //////////////////////////////////////////////
    : // 6: /drivers/net/wan/sdla.c::sdla_xfer //
    : //////////////////////////////////////////////
    :
    I am not a maintainer of sdla driver, but being Cc'd with this
    mail, I'll try to look at it.

    : - tainted signed scalar mem.len passed to kmalloc and memset (1206 and
    : 1211, or 1220 and 1223). Possibly minor because of kmalloc's
    : implicit size check

    Yes. The value mem.len is passed to kmalloc and the code immediately
    returns -ENOMEM when kmalloc fails.

    : Protected by NET_ADMIN caps, but definately needs some bound checking.
    :
    Depends on whether the kmalloc's internal checks are
    sufficient or not.

    : Jan, I think SDLA_MAX_DATA is the correct bound to check for here, can
    : you confirm please?
    :
    I cannot because I don't know or even have this hardware.
    However: looking at the definitions in include/linux/sdla.h and the
    code itself it looks like SDLA_READMEM, SDLA_WRITEMEM, and SDLA_CLEAR
    ioctls are for reading/writing/clearing the card memory itself (maybe
    for debugging purposes or downloading a firmware or what). So no,
    SDLA_MAX_DATA is probably not a correct limit there.

    The SDLA_CLEAR ioctl (the sdla_clear(dev) function) tries
    to clear exactly 65536 bytes (hardcoded at sdla.c:sdla_clear() line 140).
    So the mem.len should be <= 65536 bytes, and even mem.addr + mem.len
    should be <= 65536 bytes.

    So I propose the following patch (maybe the constant 65536 should
    be defined in sdla.h and used both in sdla_xfer() and sdla_clear()):

    Signed-off-by: Jan "Yenya" Kasprzak <kas@fi.muni.cz>

    --- linux-2.4.28/drivers/net/wan/sdla.c.orig 2002-11-29 00:53:14.000000000 +0100
    +++ linux-2.4.28/drivers/net/wan/sdla.c 2005-01-06 10:14:21.115490248 +0100
    @@ -1195,6 +1195,10 @@

    if(copy_from_user(&mem, info, sizeof(mem)))
    return -EFAULT;
    +
    + if (mem.len <= 0 || mem.addr < 0 || mem.len > 65536 || mem.addr > 65535
    + || mem.addr + mem.len > 65536)
    + return -EFAULT;

    if (read)
    {

    --
    | Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
    | GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
    | http://www.fi.muni.cz/~kas/ Czech Linux Homepage: http://www.linux.cz/ |
    > Whatever the Java applications and desktop dances may lead to, Unix will <
    > still be pushing the packets around for a quite a while. --Rob Pike <
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:09    [W:0.030 / U:0.700 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site