Skip to content

Commit

Permalink
xwayland: fix a possible clipboard race condition (#9394)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nnyyxxxx
nnyyxxxx authored Feb 15, 2025
1 parent 410da2e commit 9228116
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 25 deletions.
15 changes: 8 additions & 7 deletions src/xwayland/XDataSource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,20 @@ void CXDataSource::send(const std::string& mime, CFileDescriptor fd) {

Debug::log(LOG, "[XDataSource] send with mime {} to fd {}", mime, fd.get());

selection.transfer = makeUnique<SXTransfer>(selection);
selection.transfer->incomingWindow = xcb_generate_id(g_pXWayland->pWM->connection);
const uint32_t MASK = XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY | XCB_EVENT_MASK_PROPERTY_CHANGE;
xcb_create_window(g_pXWayland->pWM->connection, XCB_COPY_FROM_PARENT, selection.transfer->incomingWindow, g_pXWayland->pWM->screen->root, 0, 0, 10, 10, 0,
XCB_WINDOW_CLASS_INPUT_OUTPUT, g_pXWayland->pWM->screen->root_visual, XCB_CW_EVENT_MASK, &MASK);
auto transfer = makeUnique<SXTransfer>(selection);
transfer->incomingWindow = xcb_generate_id(g_pXWayland->pWM->connection);
const uint32_t MASK = XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY | XCB_EVENT_MASK_PROPERTY_CHANGE;
xcb_create_window(g_pXWayland->pWM->connection, XCB_COPY_FROM_PARENT, transfer->incomingWindow, g_pXWayland->pWM->screen->root, 0, 0, 10, 10, 0, XCB_WINDOW_CLASS_INPUT_OUTPUT,
g_pXWayland->pWM->screen->root_visual, XCB_CW_EVENT_MASK, &MASK);

xcb_convert_selection(g_pXWayland->pWM->connection, selection.transfer->incomingWindow, HYPRATOMS["CLIPBOARD"], mimeAtom, HYPRATOMS["_WL_SELECTION"], XCB_TIME_CURRENT_TIME);
xcb_convert_selection(g_pXWayland->pWM->connection, transfer->incomingWindow, HYPRATOMS["CLIPBOARD"], mimeAtom, HYPRATOMS["_WL_SELECTION"], XCB_TIME_CURRENT_TIME);

xcb_flush(g_pXWayland->pWM->connection);

//TODO: make CFileDescriptor setflags take SETFL aswell
fcntl(fd.get(), F_SETFL, O_WRONLY | O_NONBLOCK);
selection.transfer->wlFD = std::move(fd);
transfer->wlFD = std::move(fd);
selection.transfers.emplace_back(std::move(transfer));
}

void CXDataSource::accepted(const std::string& mime) {
Expand Down
81 changes: 64 additions & 17 deletions src/xwayland/XWM.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,10 @@ void CXWM::handleSelectionNotify(xcb_selection_notify_event_t* e) {
SXSelection* sel = getSelection(e->selection);

if (e->property == XCB_ATOM_NONE) {
if (sel->transfer) {
auto it = std::ranges::find_if(sel->transfers, [](const auto& t) { return !t->propertyReply; });
if (it != sel->transfers.end()) {
Debug::log(TRACE, "[xwm] converting selection failed");
sel->transfer.reset();
sel->transfers.erase(it);
}
} else if (e->target == HYPRATOMS["TARGETS"] && sel == &clipboard) {
if (!focusedSurface) {
Expand All @@ -582,7 +583,7 @@ void CXWM::handleSelectionNotify(xcb_selection_notify_event_t* e) {
}

setClipboardToWayland(*sel);
} else if (sel->transfer)
} else if (!sel->transfers.empty())
getTransferData(*sel);
}

Expand Down Expand Up @@ -1177,17 +1178,50 @@ static int writeDataSource(int fd, uint32_t mask, void* data) {
void CXWM::getTransferData(SXSelection& sel) {
Debug::log(LOG, "[xwm] getTransferData");

sel.transfer->getIncomingSelectionProp(true);
auto it = std::ranges::find_if(sel.transfers, [](const auto& t) { return !t->propertyReply; });
if (it == sel.transfers.end()) {
Debug::log(ERR, "[xwm] No pending transfer found");
return;
}

auto& transfer = *it;
if (!transfer || !transfer->incomingWindow) {
Debug::log(ERR, "[xwm] Invalid transfer state");
sel.transfers.erase(it);
return;
}

if (!transfer->getIncomingSelectionProp(true)) {
Debug::log(ERR, "[xwm] Failed to get property data");
sel.transfers.erase(it);
return;
}

if (!transfer->propertyReply) {
Debug::log(ERR, "[xwm] No property reply");
sel.transfers.erase(it);
return;
}

if (sel.transfer->propertyReply->type == HYPRATOMS["INCR"]) {
if (transfer->propertyReply->type == HYPRATOMS["INCR"]) {
Debug::log(ERR, "[xwm] Transfer is INCR, which we don't support :(");
sel.transfer.reset();
sel.transfers.erase(it);
return;
} else {
sel.onWrite();
if (sel.transfer)
sel.transfer->eventSource = wl_event_loop_add_fd(g_pCompositor->m_sWLEventLoop, sel.transfer->wlFD.get(), WL_EVENT_WRITABLE, ::writeDataSource, &sel);
}

const size_t pos = std::distance(sel.transfers.begin(), it);
sel.onWrite();

if (pos >= sel.transfers.size())
return;

auto newIt = sel.transfers.begin() + pos;
if (newIt == sel.transfers.end() || !(*newIt))
return;

auto& updatedTransfer = *newIt;
if (updatedTransfer->eventSource && updatedTransfer->wlFD.get() != -1)
updatedTransfer->eventSource = wl_event_loop_add_fd(g_pCompositor->m_sWLEventLoop, updatedTransfer->wlFD.get(), WL_EVENT_WRITABLE, ::writeDataSource, &sel);
}

void CXWM::setCursor(unsigned char* pixData, uint32_t stride, const Vector2D& size, const Vector2D& hotspot) {
Expand Down Expand Up @@ -1290,16 +1324,21 @@ void SXSelection::onKeyboardFocus() {
}

int SXSelection::onRead(int fd, uint32_t mask) {
// TODO: support INCR
auto it = std::ranges::find_if(transfers, [fd](const auto& t) { return t->wlFD.get() == fd; });
if (it == transfers.end()) {
Debug::log(ERR, "[xwm] No transfer found for fd {}", fd);
return 0;
}

size_t pre = transfer->data.size();
auto& transfer = *it;
size_t pre = transfer->data.size();
transfer->data.resize(INCR_CHUNK_SIZE + pre);

auto len = read(fd, transfer->data.data() + pre, INCR_CHUNK_SIZE - 1);
if (len < 0) {
Debug::log(ERR, "[xwm] readDataSource died");
g_pXWayland->pWM->selectionSendNotify(&transfer->request, false);
transfer.reset();
transfers.erase(it);
return 0;
}

Expand All @@ -1311,7 +1350,7 @@ int SXSelection::onRead(int fd, uint32_t mask) {
transfer->data.size(), transfer->data.data());
xcb_flush(g_pXWayland->pWM->connection);
g_pXWayland->pWM->selectionSendNotify(&transfer->request, true);
transfer.reset();
transfers.erase(it);
} else
Debug::log(LOG, "[xwm] Received {} bytes, waiting...", len);

Expand Down Expand Up @@ -1346,7 +1385,7 @@ bool SXSelection::sendData(xcb_selection_request_event_t* e, std::string mime) {
mime = *MIMES.begin();
}

transfer = makeUnique<SXTransfer>(*this);
auto transfer = makeUnique<SXTransfer>(*this);
transfer->request = *e;

int p[2];
Expand All @@ -1368,18 +1407,26 @@ bool SXSelection::sendData(xcb_selection_request_event_t* e, std::string mime) {
selection->send(mime, CFileDescriptor{p[1]});

transfer->eventSource = wl_event_loop_add_fd(g_pCompositor->m_sWLEventLoop, transfer->wlFD.get(), WL_EVENT_READABLE, ::readDataSource, this);
transfers.emplace_back(std::move(transfer));

return true;
}

int SXSelection::onWrite() {
auto it = std::ranges::find_if(transfers, [](const auto& t) { return t->propertyReply; });
if (it == transfers.end()) {
Debug::log(ERR, "[xwm] No transfer with property data found");
return 0;
}

auto& transfer = *it;
char* property = (char*)xcb_get_property_value(transfer->propertyReply);
int remainder = xcb_get_property_value_length(transfer->propertyReply) - transfer->propertyStart;

ssize_t len = write(transfer->wlFD.get(), property + transfer->propertyStart, remainder);
if (len == -1) {
Debug::log(ERR, "[xwm] write died in transfer get");
transfer.reset();
transfers.erase(it);
return 0;
}

Expand All @@ -1388,7 +1435,7 @@ int SXSelection::onWrite() {
Debug::log(LOG, "[xwm] wl client read partially: len {}", len);
} else {
Debug::log(LOG, "[xwm] cb transfer to wl client complete, read {} bytes", len);
transfer.reset();
transfers.erase(it);
}

return 1;
Expand Down
2 changes: 1 addition & 1 deletion src/xwayland/XWM.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct SXSelection {
CHyprSignalListener keyboardFocusChange;
} listeners;

UP<SXTransfer> transfer;
std::vector<UP<SXTransfer>> transfers;
};

class CXCBConnection {
Expand Down

1 comment on commit 9228116

@nnyyxxxx
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commit title is wrong, it wasnt a "possible" race condition, it was very much a race condition

Clearly 2 listeners would try to intercept the transfer, while only 1 could receive it...

Please sign in to comment.