From bef843b97f2e91859f2a08845822d619e769f89f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 Oct 2019 18:20:32 +0200 Subject: [PATCH] MDEV-20917 InnoDB is passing NULL to nonnull function parameters mem_heap_dup(): Avoid mem_heap_alloc() and memcpy() of data=NULL, len=0. trx_undo_report_insert_virtual(), trx_undo_page_report_insert(), trx_undo_page_report_modify(): Avoid memcpy(ptr, NULL, 0). dfield_data_is_binary_equal(): Correctly handle data=NULL, len=0. rec_init_offsets_temp(): Do allow def_val=NULL in the interface. This clean-up was motivated by WITH_UBSAN, and no bug related to this was observed in the wild. It should be noted that undefined behaviour such as memcpy(ptr, NULL, 0) could allow compilers to perform unsafe optimizations, like it was the case in commit fc168c3a5e58d8b364a2e87e0d876a261ec7fced (MDEV-15587). --- storage/innobase/include/data0data.h | 2 +- storage/innobase/include/data0data.ic | 2 +- storage/innobase/include/mem0mem.h | 5 +++- storage/innobase/include/rem0rec.h | 2 +- storage/innobase/mem/mem0mem.cc | 2 +- storage/innobase/trx/trx0rec.cc | 40 +++++++++++++++------------ 6 files changed, 31 insertions(+), 22 deletions(-) diff --git a/storage/innobase/include/data0data.h b/storage/innobase/include/data0data.h index fbbdd3b8f2d..706d65a4469 100644 --- a/storage/innobase/include/data0data.h +++ b/storage/innobase/include/data0data.h @@ -168,7 +168,7 @@ dfield_data_is_binary_equal( const dfield_t* field, /*!< in: field */ ulint len, /*!< in: data length or UNIV_SQL_NULL */ const byte* data) /*!< in: data */ - MY_ATTRIBUTE((nonnull, warn_unused_result)); + MY_ATTRIBUTE((nonnull(1), warn_unused_result)); /*********************************************************************//** Gets info bits in a data tuple. diff --git a/storage/innobase/include/data0data.ic b/storage/innobase/include/data0data.ic index be0186d53fe..92be8f8c589 100644 --- a/storage/innobase/include/data0data.ic +++ b/storage/innobase/include/data0data.ic @@ -225,7 +225,7 @@ dfield_data_is_binary_equal( { ut_ad(len != UNIV_SQL_DEFAULT); return(len == dfield_get_len(field) - && (len == UNIV_SQL_NULL + && (!len || len == UNIV_SQL_NULL || !memcmp(dfield_get_data(field), data, len))); } diff --git a/storage/innobase/include/mem0mem.h b/storage/innobase/include/mem0mem.h index 6d0f95cba19..fa22b3d3086 100644 --- a/storage/innobase/include/mem0mem.h +++ b/storage/innobase/include/mem0mem.h @@ -237,7 +237,10 @@ inline void* mem_heap_dup(mem_heap_t* heap, const void* data, size_t len) { - return(memcpy(mem_heap_alloc(heap, len), data, len)); + ut_ad(data || !len); + return UNIV_LIKELY(data != NULL) + ? memcpy(mem_heap_alloc(heap, len), data, len) + : NULL; } /** Duplicate a NUL-terminated string, allocated from a memory heap. diff --git a/storage/innobase/include/rem0rec.h b/storage/innobase/include/rem0rec.h index 54f50058140..0c49c223e10 100644 --- a/storage/innobase/include/rem0rec.h +++ b/storage/innobase/include/rem0rec.h @@ -983,7 +983,7 @@ rec_init_offsets_temp( ulint n_core, const dict_col_t::def_t*def_val, rec_comp_status_t status = REC_STATUS_ORDINARY) - MY_ATTRIBUTE((nonnull)); + MY_ATTRIBUTE((nonnull(1,2,3))); /** Determine the offset to each field in temporary file. @param[in] rec temporary file record @param[in] index index of that the record belongs to diff --git a/storage/innobase/mem/mem0mem.cc b/storage/innobase/mem/mem0mem.cc index 929bffb881c..759308faac8 100644 --- a/storage/innobase/mem/mem0mem.cc +++ b/storage/innobase/mem/mem0mem.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1994, 2014, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, 2018, MariaDB Corporation. +Copyright (c) 2017, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index 44b9f056770..901dd90c005 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -421,9 +421,15 @@ trx_undo_report_insert_virtual( const dfield_t* vfield = dtuple_get_nth_v_field( row, col->v_pos); - ulint flen = vfield->len; + switch (ulint flen = vfield->len) { + case 0: case UNIV_SQL_NULL: + if (trx_undo_left(undo_block, *ptr) < 5) { + return(false); + } - if (flen != UNIV_SQL_NULL) { + *ptr += mach_write_compressed(*ptr, flen); + break; + default: ulint max_len = dict_max_v_field_len_store_undo( table, col_no); @@ -438,14 +444,8 @@ trx_undo_report_insert_virtual( } *ptr += mach_write_compressed(*ptr, flen); - ut_memcpy(*ptr, vfield->data, flen); + memcpy(*ptr, vfield->data, flen); *ptr += flen; - } else { - if (trx_undo_left(undo_block, *ptr) < 5) { - return(false); - } - - *ptr += mach_write_compressed(*ptr, flen); } } } @@ -524,13 +524,16 @@ trx_undo_page_report_insert( ptr += mach_write_compressed(ptr, flen); - if (flen != UNIV_SQL_NULL) { + switch (flen) { + case 0: case UNIV_SQL_NULL: + break; + default: if (trx_undo_left(undo_block, ptr) < flen) { return(0); } - ut_memcpy(ptr, dfield_get_data(field), flen); + memcpy(ptr, dfield_get_data(field), flen); ptr += flen; } } @@ -998,7 +1001,7 @@ trx_undo_page_report_modify( return(0); } - ut_memcpy(ptr, field, flen); + memcpy(ptr, field, flen); ptr += flen; } } @@ -1129,7 +1132,7 @@ trx_undo_page_report_modify( return(0); } - ut_memcpy(ptr, field, flen); + memcpy(ptr, field, flen); ptr += flen; } @@ -1154,7 +1157,7 @@ trx_undo_page_report_modify( return(0); } - ut_memcpy(ptr, field, flen); + memcpy(ptr, field, flen); ptr += flen; } } @@ -1303,7 +1306,7 @@ trx_undo_page_report_modify( return(0); } - ut_memcpy(ptr, field, flen); + memcpy(ptr, field, flen); ptr += flen; } @@ -1388,13 +1391,16 @@ already_logged: ptr += mach_write_compressed(ptr, flen); - if (flen != UNIV_SQL_NULL) { + switch (flen) { + case 0: case UNIV_SQL_NULL: + break; + default: if (trx_undo_left(undo_block, ptr) < flen) { return(0); } - ut_memcpy(ptr, field, flen); + memcpy(ptr, field, flen); ptr += flen; } }