1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QHttp1Configuration: fix UB (inactive union member access)

Browse Source 
Any QHttp1Configuration object created has 'ShortData data' as active
member. std::exchange()ing or qt_ptr_swap()ping 'Private *d', then, is
accessing the inactive member of the union, and that's UB.

Fix by swapping and copying the whole union, as opposed to any of its
members, and, in the move constructor, activating Private *d.

This is now safe, as assigning to 'd' ends 'data's life-time and
starts 'd's. Even if we assign a well-formed object to a moved-from
object, we either swap or copy the whole union, so SEP. For
self-move-assignment in the moved-from state (Hinnant Criterion),
we're using std::swap() on the whole union, so SEP.

In addition, activating `Private *d` in moved-from objects means that
a future ~Public's use of unconditional 'delete d' won't invoke UB,
either.

Thanks to Peppe for insisting on fixing this.

Change-Id: Ic1323b8416d6b17ae21768c625de1daba0944133
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 47abdabe2cfd20715b0186b9c257bc32db9315bb)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2023-01-17 14:09:22 +01:00 committed by Qt Cherry-pick Bot
parent c2e1d4a791
commit 26bef6ec2e
2 changed files with 10 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/network/access/qhttp1configuration.cpp
View File

@ -36,7 +36,7 @@ static_assert(sizeof(QHttp1Configuration) == sizeof(void*),
Default constructs a QHttp1Configuration object.
*/
QHttp1Configuration::QHttp1Configuration()
: data{6, {}} // QHttpNetworkConnectionPrivate::defaultHttpChannelCount
: u(ShortData{6, {}}) // QHttpNetworkConnectionPrivate::defaultHttpChannelCount
{
}
@ -91,7 +91,7 @@ void QHttp1Configuration::setNumberOfConnectionsPerHost(qsizetype number)
auto n = qt_saturate<std::uint8_t>(number);
if (n == 0)
return;
data.numConnectionsPerHost = n;
u.data.numConnectionsPerHost = n;
}
/*!
@ -102,7 +102,7 @@ void QHttp1Configuration::setNumberOfConnectionsPerHost(qsizetype number)
*/
qsizetype QHttp1Configuration::numberOfConnectionsPerHost() const
{
return data.numConnectionsPerHost;
return u.data.numConnectionsPerHost;
}
/*!
@ -140,7 +140,7 @@ qsizetype QHttp1Configuration::numberOfConnectionsPerHost() const
*/
bool QHttp1Configuration::equals(const QHttp1Configuration &other) const noexcept
{
return data.numConnectionsPerHost == other.data.numConnectionsPerHost;
return u.data.numConnectionsPerHost == other.u.data.numConnectionsPerHost;
}
/*!
@ -148,7 +148,7 @@ bool QHttp1Configuration::equals(const QHttp1Configuration &other) const noexcep
*/
size_t QHttp1Configuration::hash(size_t seed) const noexcept
{
return qHash(data.numConnectionsPerHost, seed);
return qHash(u.data.numConnectionsPerHost, seed);
}
QT_END_NAMESPACE

9
src/network/access/qhttp1configuration.h
View File

@ -22,7 +22,7 @@ public:
Q_NETWORK_EXPORT QHttp1Configuration();
Q_NETWORK_EXPORT QHttp1Configuration(const QHttp1Configuration &other);
QHttp1Configuration(QHttp1Configuration &&other) noexcept
: d{std::exchange(other.d, nullptr)} {}
: u{other.u} { other.u.d = nullptr; }
Q_NETWORK_EXPORT QHttp1Configuration &operator=(const QHttp1Configuration &other);
QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QHttp1Configuration)
@ -33,17 +33,18 @@ public:
Q_NETWORK_EXPORT qsizetype numberOfConnectionsPerHost() const;
void swap(QHttp1Configuration &other) noexcept
{ qt_ptr_swap(d, other.d); }
{ std::swap(u, other.u); }
private:
struct ShortData {
std::uint8_t numConnectionsPerHost;
char reserved[sizeof(void*) - sizeof(numConnectionsPerHost)];
};
union {
union U {
U(ShortData _data) : data(_data) {}
QHttp1ConfigurationPrivate *d;
ShortData data;
};
} u;
Q_NETWORK_EXPORT bool equals(const QHttp1Configuration &other) const noexcept;
Q_NETWORK_EXPORT size_t hash(size_t seed) const noexcept;