lkml.org 
[lkml]   [2021]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 139/167] media: ngene: Fix out-of-bounds bug in ngene_command_config_free_buf()
    Date
    From: Gustavo A. R. Silva <gustavoars@kernel.org>

    commit 8d4abca95ecc82fc8c41912fa0085281f19cc29f upstream.

    Fix an 11-year old bug in ngene_command_config_free_buf() while
    addressing the following warnings caught with -Warray-bounds:

    arch/alpha/include/asm/string.h:22:16: warning: '__builtin_memcpy' offset [12, 16] from the object at 'com' is out of the bounds of referenced subobject 'config' with type 'unsigned char' at offset 10 [-Warray-bounds]
    arch/x86/include/asm/string_32.h:182:25: warning: '__builtin_memcpy' offset [12, 16] from the object at 'com' is out of the bounds of referenced subobject 'config' with type 'unsigned char' at offset 10 [-Warray-bounds]

    The problem is that the original code is trying to copy 6 bytes of
    data into a one-byte size member _config_ of the wrong structue
    FW_CONFIGURE_BUFFERS, in a single call to memcpy(). This causes a
    legitimate compiler warning because memcpy() overruns the length
    of &com.cmd.ConfigureBuffers.config. It seems that the right
    structure is FW_CONFIGURE_FREE_BUFFERS, instead, because it contains
    6 more members apart from the header _hdr_. Also, the name of
    the function ngene_command_config_free_buf() suggests that the actual
    intention is to ConfigureFreeBuffers, instead of ConfigureBuffers
    (which takes place in the function ngene_command_config_buf(), above).

    Fix this by enclosing those 6 members of struct FW_CONFIGURE_FREE_BUFFERS
    into new struct config, and use &com.cmd.ConfigureFreeBuffers.config as
    the destination address, instead of &com.cmd.ConfigureBuffers.config,
    when calling memcpy().

    This also helps with the ongoing efforts to globally enable
    -Warray-bounds and get us closer to being able to tighten the
    FORTIFY_SOURCE routines on memcpy().

    Link: https://github.com/KSPP/linux/issues/109
    Fixes: dae52d009fc9 ("V4L/DVB: ngene: Initial check-in")
    Cc: stable@vger.kernel.org
    Reported-by: kernel test robot <lkp@intel.com>
    Reviewed-by: Kees Cook <keescook@chromium.org>
    Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
    Link: https://lore.kernel.org/linux-hardening/20210420001631.GA45456@embeddedor/
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/media/pci/ngene/ngene-core.c | 2 +-
    drivers/media/pci/ngene/ngene.h | 14 ++++++++------
    2 files changed, 9 insertions(+), 7 deletions(-)

    --- a/drivers/media/pci/ngene/ngene-core.c
    +++ b/drivers/media/pci/ngene/ngene-core.c
    @@ -385,7 +385,7 @@ static int ngene_command_config_free_buf

    com.cmd.hdr.Opcode = CMD_CONFIGURE_FREE_BUFFER;
    com.cmd.hdr.Length = 6;
    - memcpy(&com.cmd.ConfigureBuffers.config, config, 6);
    + memcpy(&com.cmd.ConfigureFreeBuffers.config, config, 6);
    com.in_len = 6;
    com.out_len = 0;

    --- a/drivers/media/pci/ngene/ngene.h
    +++ b/drivers/media/pci/ngene/ngene.h
    @@ -407,12 +407,14 @@ enum _BUFFER_CONFIGS {

    struct FW_CONFIGURE_FREE_BUFFERS {
    struct FW_HEADER hdr;
    - u8 UVI1_BufferLength;
    - u8 UVI2_BufferLength;
    - u8 TVO_BufferLength;
    - u8 AUD1_BufferLength;
    - u8 AUD2_BufferLength;
    - u8 TVA_BufferLength;
    + struct {
    + u8 UVI1_BufferLength;
    + u8 UVI2_BufferLength;
    + u8 TVO_BufferLength;
    + u8 AUD1_BufferLength;
    + u8 AUD2_BufferLength;
    + u8 TVA_BufferLength;
    + } __packed config;
    } __attribute__ ((__packed__));

    struct FW_CONFIGURE_UART {

    \
     
     \ /
      Last update: 2021-07-26 18:27    [W:4.191 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site