lkml.org 
[lkml]   [2019]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH 4.9 45/83] staging: vc04_services: prevent integer overflow in create_pagelist()
    Date
    Hi.

    On 6/9/19 6:42 PM, Greg Kroah-Hartman wrote:
    > From: Dan Carpenter <dan.carpenter@oracle.com>
    >
    > commit ca641bae6da977d638458e78cd1487b6160a2718 upstream.

    This commit breaks the kernel build because the vchiq_pagelist_info
    struct is not defined in v4.9.182.

    It was only added in v4.10, in commit
    4807f2c0e684e907c501cb96049809d7a957dbc2.


    Best regards,

    Martin Weinelt


    In file included from ./include/uapi/linux/posix_types.h:4:0,
    from ./include/uapi/linux/types.h:13,
    from ./include/linux/compiler.h:224,
    from ./include/linux/linkage.h:4,
    from ./include/linux/kernel.h:6,
    from
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:34:
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c: In
    function 'create_pagelist':
    ./include/linux/stddef.h:7:14: warning: return makes integer from
    pointer without a cast [-Wint-conversion]
    #define NULL ((void *)0)
    ^
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:385:10:
    note: in expansion of macro 'NULL'
    return NULL;
    ^~~~
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:391:12:
    error: invalid application of 'sizeof' to incomplete type 'struct
    vchiq_pagelist_info'
    sizeof(struct vchiq_pagelist_info)) /
    ^~~~~~
    In file included from ./include/uapi/linux/posix_types.h:4:0,
    from ./include/uapi/linux/types.h:13,
    from ./include/linux/compiler.h:224,
    from ./include/linux/linkage.h:4,
    from ./include/linux/kernel.h:6,
    from
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:34:
    ./include/linux/stddef.h:7:14: warning: return makes integer from
    pointer without a cast [-Wint-conversion]
    #define NULL ((void *)0)
    ^
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:394:10:
    note: in expansion of macro 'NULL'
    return NULL;
    ^~~~


    >
    > The create_pagelist() "count" parameter comes from the user in
    > vchiq_ioctl() and it could overflow. If you look at how create_page()
    > is called in vchiq_prepare_bulk_data(), then the "size" variable is an
    > int so it doesn't make sense to allow negatives or larger than INT_MAX.
    >
    > I don't know this code terribly well, but I believe that typical values
    > of "count" are typically quite low and I don't think this check will
    > affect normal valid uses at all.
    >
    > The "pagelist_size" calculation can also overflow on 32 bit systems, but
    > not on 64 bit systems. I have added an integer overflow check for that
    > as well.
    >
    > The Raspberry PI doesn't offer the same level of memory protection that
    > x86 does so these sorts of bugs are probably not super critical to fix.
    >
    > Fixes: 71bad7f08641 ("staging: add bcm2708 vchiq driver")
    > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
    > Cc: stable <stable@vger.kernel.org>
    > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > ---
    > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 9 +++++++++
    > 1 file changed, 9 insertions(+)
    >
    > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
    > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
    > @@ -381,9 +381,18 @@ create_pagelist(char __user *buf, size_t
    > int run, addridx, actual_pages;
    > unsigned long *need_release;
    >
    > + if (count >= INT_MAX - PAGE_SIZE)
    > + return NULL;
    > +
    > offset = (unsigned int)buf & (PAGE_SIZE - 1);
    > num_pages = (count + offset + PAGE_SIZE - 1) / PAGE_SIZE;
    >
    > + if (num_pages > (SIZE_MAX - sizeof(PAGELIST_T) -
    > + sizeof(struct vchiq_pagelist_info)) /
    > + (sizeof(u32) + sizeof(pages[0]) +
    > + sizeof(struct scatterlist)))
    > + return NULL;
    > +
    > *ppagelist = NULL;
    >
    > /* Allocate enough storage to hold the page pointers and the page
    >

    \
     
     \ /
      Last update: 2019-06-19 18:11    [W:4.026 / U:0.180 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site