Re: [PATCH] libstdc++: avoid uninitialized read in basic_string constructor

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 2 Nov 2023 at 19:58, Ben Sherman <ben.sherman@chicagotrading.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, please let me know if there's anything
> else needed. I haven't contributed before and don't have write access, so
> apologies if I've missed anything.

This was https://gcc.gnu.org/PR109703 (and several duplicates) and
should already be fixed in all affected branches. Where are you seeing
this?

> The basic_string input iterator constructor incrementally reads data and
> allocates the internal buffer as-needed. When _M_dispose() is called, there
> is a check for whether the local buffer is being used - if it is, there is
> an additional check guarding __builtin_unreachable() for the value of
> _M_string_length. The constructor does not initialize _M_string_length
> until all data has been read, so the first re-allocation out of the local
> buffer will have an uninitialized read.
>
> This updates the basic_string input iterator constructor to properly set
> _M_string_length as data is being read.  It additionally introduces a new
> _M_assign_terminator() function to assign the null-terminator based on the
> currently-stored _M_string_length.

Adding new member functions to std::string requires exporting them
from the shared library, which requires bumping the shared library
version, which is an ABI change that isn't suitable for backporting to
release branches. But it doesn't matter if we don't need to make this
change (and I don't think we do need to).


> libstdc++-v3/ChangeLog:
>
>         * include/bits/basic_string.h (_M_assign_terminator()): New
>           function.
>           (_M_set_length()): Use _M_assign_terminator().
>         * include/bits/basic_string.tcc (_M_construct(InIter, InIter,
>           input_iterator_tag)): Set length incrementally, use
>           _M_assign_terminator().
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index 0fa32afeb..ba02d8f0f 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -258,12 +258,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _M_capacity(size_type __capacity)
>        { _M_allocated_capacity = __capacity; }
>
> +      _GLIBCXX20_CONSTEXPR
> +      void
> +      _M_assign_terminator()
> +      { traits_type::assign(_M_data()[_M_string_length], _CharT()); }
> +
>        _GLIBCXX20_CONSTEXPR
>        void
>        _M_set_length(size_type __n)
>        {
>         _M_length(__n);
> -       traits_type::assign(_M_data()[__n], _CharT());
> +       _M_assign_terminator();
>        }
>
>        _GLIBCXX20_CONSTEXPR
> diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
> index f0a44e5e8..84366a44a 100644
> --- a/libstdc++-v3/include/bits/basic_string.tcc
> +++ b/libstdc++-v3/include/bits/basic_string.tcc
> @@ -182,6 +182,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>             ++__beg;
>           }
>
> +       _M_length(__len);
> +
>         struct _Guard
>         {
>           _GLIBCXX20_CONSTEXPR
> @@ -206,12 +208,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>                 _M_capacity(__capacity);
>               }
>             traits_type::assign(_M_data()[__len++], *__beg);
> +           _M_length(__len);
>             ++__beg;
>           }
>
>         __guard._M_guarded = 0;
>
> -       _M_set_length(__len);
> +       _M_assign_terminator();
>        }
>
>    template<typename _CharT, typename _Traits, typename _Alloc>
> --
> 2.21.0
>
>
>
>
>
>
>
> This electronic mail message and any attached files contain information intended for the exclusive use of the individual or entity to whom it is addressed and may contain information that is proprietary, confidential and/or exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any viewing, copying, disclosure or distribution of this information may be subject to legal restriction or sanction. Please notify the sender, by electronic mail or telephone, of any unintended recipients and delete the original message without making any copies.
>