From c0ebec51e3dd1b52767878fe7fd56ce6e8f95461 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Wed, 16 Jan 2019 15:12:16 +0800 Subject: xcb: respect big-request encoding in max request size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From big-request specification: "This extension defines a mechanism for extending the length field beyond 16 bits. If the normal 16-bit length field of the protocol request is zero, then an additional 32-bit field containing the actual length (in 4-byte units) is inserted into the request, immediately following the 16-bit length field." Meaning that the request requires 4 additional bytes. This patch provides a convenience API for calculating maximum request data size. Besides fixing QTBUG-73044, it was also discovered that calculations for xcb_image_put (in QXcbBackingStoreImage::flushPixmap) were wrong. The code assumed that xcb_get_maximum_request_length() returns bytes, but what it actually returns is length which is measured in four-byte units. This means that we were sending 4x less bytes than allowed by the protocol. Furthermore, use the actual 'stride' (bytes per line) value when calculating rows_per_put. The new stride value was introduced by 760b2929a3b268e2edf14a561329bdb78fbdc26e, but was not updated in rows_per_put calculations. Fixes: QTBUG-73044 Done-with: JiDe Zhang Done-with: Mikhail Svetkin Change-Id: I06beb6082da3e8bc78225a87603914e796fe5878 Reviewed-by: Błażej Szczygieł Reviewed-by: JiDe Zhang Reviewed-by: Mikhail Svetkin Reviewed-by: Uli Schlachter Reviewed-by: Allan Sandfeld Jensen --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 17 ++++++++-------- src/plugins/platforms/xcb/qxcbclipboard.cpp | 23 ++++++++-------------- src/plugins/platforms/xcb/qxcbclipboard.h | 4 ++-- src/plugins/platforms/xcb/qxcbconnection.h | 2 +- src/plugins/platforms/xcb/qxcbconnection_basic.cpp | 14 +++++++++++++ src/plugins/platforms/xcb/qxcbconnection_basic.h | 5 +++++ 6 files changed, 39 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index f9240a45cc..741317d766 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -641,17 +641,17 @@ void QXcbBackingStoreImage::flushPixmap(const QRegion ®ion, bool fullRegion) xcb_subimage.bit_order = m_xcb_image->bit_order; const bool needsByteSwap = xcb_subimage.byte_order != m_xcb_image->byte_order; + // Ensure that we don't send more than maxPutImageRequestDataBytes per request. + const auto maxPutImageRequestDataBytes = connection()->maxRequestDataBytes(sizeof(xcb_put_image_request_t)); for (const QRect &rect : region) { - // We must make sure that each request is not larger than max_req_size. - // Each request takes req_size + m_xcb_image->stride * height bytes. - static const uint32_t req_size = sizeof(xcb_put_image_request_t); - const uint32_t max_req_size = xcb_get_maximum_request_length(xcb_connection()); - const int rows_per_put = (max_req_size - req_size) / m_xcb_image->stride; + const quint32 stride = round_up_scanline(rect.width() * m_qimage.depth(), xcb_subimage.scanline_pad) >> 3; + const int rows_per_put = maxPutImageRequestDataBytes / stride; // This assert could trigger if a single row has more pixels than fit in - // a single PutImage request. However, max_req_size is guaranteed to be - // at least 16384 bytes. That should be enough for quite large images. + // a single PutImage request. In the absence of the BIG-REQUESTS extension + // the theoretical maximum lengths of maxPutImageRequestDataBytes can be + // roughly 256kB. Q_ASSERT(rows_per_put > 0); // If we upload the whole image in a single chunk, the result might be @@ -666,9 +666,10 @@ void QXcbBackingStoreImage::flushPixmap(const QRegion ®ion, bool fullRegion) while (height > 0) { const int rows = std::min(height, rows_per_put); const QRect subRect(x, y, width, rows); - const quint32 stride = round_up_scanline(width * m_qimage.depth(), xcb_subimage.scanline_pad) >> 3; const QImage subImage = native_sub_image(&m_flushBuffer, stride, m_qimage, subRect, needsByteSwap); + Q_ASSERT(static_cast(subImage.sizeInBytes()) <= maxPutImageRequestDataBytes); + xcb_subimage.width = width; xcb_subimage.height = rows; xcb_subimage.data = const_cast(subImage.constBits()); diff --git a/src/plugins/platforms/xcb/qxcbclipboard.cpp b/src/plugins/platforms/xcb/qxcbclipboard.cpp index ac8b029916..2cb6720d40 100644 --- a/src/plugins/platforms/xcb/qxcbclipboard.cpp +++ b/src/plugins/platforms/xcb/qxcbclipboard.cpp @@ -240,8 +240,8 @@ QXcbClipboard::QXcbClipboard(QXcbConnection *c) xcb_xfixes_select_selection_input_checked(xcb_connection(), m_owner, atom(QXcbAtom::CLIPBOARD), mask); } - // change property protocol request is 24 bytes - m_increment = (xcb_get_maximum_request_length(xcb_connection()) * 4) - 24; + // xcb_change_property_request_t and xcb_get_property_request_t are the same size + m_maxPropertyRequestDataBytes = connection()->maxRequestDataBytes(sizeof(xcb_change_property_request_t)); } QXcbClipboard::~QXcbClipboard() @@ -486,7 +486,7 @@ xcb_atom_t QXcbClipboard::sendSelection(QMimeData *d, xcb_atom_t target, xcb_win if (m_clipboard_closing) allow_incr = false; - if (data.size() > m_increment && allow_incr) { + if (data.size() > m_maxPropertyRequestDataBytes && allow_incr) { long bytes = data.size(); xcb_change_property(xcb_connection(), XCB_PROP_MODE_REPLACE, window, property, atom(QXcbAtom::INCR), 32, 1, (const void *)&bytes); @@ -496,7 +496,7 @@ xcb_atom_t QXcbClipboard::sendSelection(QMimeData *d, xcb_atom_t target, xcb_win } // make sure we can perform the XChangeProperty in a single request - if (data.size() > m_increment) + if (data.size() > m_maxPropertyRequestDataBytes) return XCB_NONE; // ### perhaps use several XChangeProperty calls w/ PropModeAppend? int dataSize = data.size() / (dataFormat / 8); // use a single request to transfer data @@ -678,17 +678,8 @@ void QXcbClipboard::handleXFixesSelectionRequest(xcb_xfixes_selection_notify_eve emitChanged(mode); } - -static inline int maxSelectionIncr(xcb_connection_t *c) -{ - int l = xcb_get_maximum_request_length(c); - return (l > 65536 ? 65536*4 : l*4) - 100; -} - bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property, bool deleteProperty, QByteArray *buffer, int *size, xcb_atom_t *type, int *format) { - int maxsize = maxSelectionIncr(xcb_connection()); - ulong bytes_left; // bytes_after xcb_atom_t dummy_type; int dummy_format; @@ -705,7 +696,8 @@ bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property, } *type = reply->type; *format = reply->format; - bytes_left = reply->bytes_after; + + auto bytes_left = reply->bytes_after; int offset = 0, buffer_offset = 0; @@ -720,7 +712,8 @@ bool QXcbClipboard::clipboardReadProperty(xcb_window_t win, xcb_atom_t property, while (bytes_left) { // more to read... - reply = Q_XCB_REPLY(xcb_get_property, xcb_connection(), false, win, property, XCB_GET_PROPERTY_TYPE_ANY, offset, maxsize/4); + reply = Q_XCB_REPLY(xcb_get_property, xcb_connection(), false, win, property, + XCB_GET_PROPERTY_TYPE_ANY, offset, m_maxPropertyRequestDataBytes / 4); if (!reply || reply->type == XCB_NONE) break; diff --git a/src/plugins/platforms/xcb/qxcbclipboard.h b/src/plugins/platforms/xcb/qxcbclipboard.h index 26d3b3b395..51ae0dc1ee 100644 --- a/src/plugins/platforms/xcb/qxcbclipboard.h +++ b/src/plugins/platforms/xcb/qxcbclipboard.h @@ -113,7 +113,7 @@ public: xcb_window_t getSelectionOwner(xcb_atom_t atom) const; QByteArray getSelection(xcb_atom_t selection, xcb_atom_t target, xcb_atom_t property, xcb_timestamp_t t = 0); - int increment() const { return m_increment; } + int increment() const { return m_maxPropertyRequestDataBytes; } int clipboardTimeout() const { return clipboard_timeout; } void removeTransaction(xcb_window_t window) { m_transactions.remove(window); } @@ -137,7 +137,7 @@ private: static const int clipboard_timeout; - int m_increment = 0; + int m_maxPropertyRequestDataBytes = 0; bool m_clipboard_closing = false; xcb_timestamp_t m_incr_receive_time = 0; diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h index d63888b48f..0e3ecd135b 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.h +++ b/src/plugins/platforms/xcb/qxcbconnection.h @@ -245,7 +245,7 @@ public: void flush() { xcb_flush(xcb_connection()); } void processXcbEvents(QEventLoop::ProcessEventsFlags flags); - QTimer &focusInTimer() { return m_focusInTimer; }; + QTimer &focusInTimer() { return m_focusInTimer; } protected: bool event(QEvent *e) override; diff --git a/src/plugins/platforms/xcb/qxcbconnection_basic.cpp b/src/plugins/platforms/xcb/qxcbconnection_basic.cpp index af72285135..9a028e5a7e 100644 --- a/src/plugins/platforms/xcb/qxcbconnection_basic.cpp +++ b/src/plugins/platforms/xcb/qxcbconnection_basic.cpp @@ -134,6 +134,7 @@ QXcbBasicConnection::QXcbBasicConnection(const char *displayName) m_setup = xcb_get_setup(m_xcbConnection); m_xcbAtom.initialize(m_xcbConnection); + m_maximumRequestLength = xcb_get_maximum_request_length(m_xcbConnection); xcb_extension_t *extensions[] = { &xcb_shm_id, &xcb_xfixes_id, &xcb_randr_id, &xcb_shape_id, &xcb_sync_id, @@ -178,6 +179,14 @@ QXcbBasicConnection::~QXcbBasicConnection() } } +size_t QXcbBasicConnection::maxRequestDataBytes(size_t requestSize) const +{ + if (hasBigRequest()) + requestSize += 4; // big-request encoding adds 4 bytes + + return m_maximumRequestLength * 4 - requestSize; +} + xcb_atom_t QXcbBasicConnection::internAtom(const char *name) { if (!name || *name == 0) @@ -199,6 +208,11 @@ QByteArray QXcbBasicConnection::atomName(xcb_atom_t atom) return QByteArray(); } +bool QXcbBasicConnection::hasBigRequest() const +{ + return m_maximumRequestLength > m_setup->maximum_request_length; +} + #if QT_CONFIG(xcb_xinput) // Starting from the xcb version 1.9.3 struct xcb_ge_event_t has changed: // - "pad0" became "extension" diff --git a/src/plugins/platforms/xcb/qxcbconnection_basic.h b/src/plugins/platforms/xcb/qxcbconnection_basic.h index ca91f7fa45..3691763398 100644 --- a/src/plugins/platforms/xcb/qxcbconnection_basic.h +++ b/src/plugins/platforms/xcb/qxcbconnection_basic.h @@ -73,6 +73,8 @@ public: } const xcb_setup_t *setup() const { return m_setup; } + size_t maxRequestDataBytes(size_t requestSize) const; + inline xcb_atom_t atom(QXcbAtom::Atom qatom) const { return m_xcbAtom.atom(qatom); } QXcbAtom::Atom qatom(xcb_atom_t atom) const { return m_xcbAtom.qatom(atom); } xcb_atom_t internAtom(const char *name); @@ -94,6 +96,7 @@ public: bool hasShmFd() const { return m_hasShmFd; } bool hasXSync() const { return m_hasXSync; } bool hasXinerama() const { return m_hasXinerama; } + bool hasBigRequest() const; #if QT_CONFIG(xcb_xinput) bool isAtLeastXI21() const { return m_xi2Enabled && m_xi2Minor >= 1; } @@ -152,6 +155,8 @@ private: uint32_t m_xfixesFirstEvent = 0; uint32_t m_xrandrFirstEvent = 0; uint32_t m_xkbFirstEvent = 0; + + uint32_t m_maximumRequestLength = 0; }; #define Q_XCB_REPLY_CONNECTION_ARG(connection, ...) connection -- cgit v1.2.3