Skip to content

memcached: apply patch: fix crash in binary protocol #18459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Chris Gunn (cwize1) wants to merge 2 commits into
base: 3.0-dev
Choose a base branch
from
+36 −1
Open

memcached: apply patch: fix crash in binary protocol #18459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ed1f73
memcached: patch: fix crash in binary protocol
cwize1 Aug 14, 2026
6301599
Feedback updates
cwize1 Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions SPECS/memcached/0001-proto-fix-crash-in-binary-protocol.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
From 2e623108dec9759ad3e258aa695288a1ef6a0c72 Mon Sep 17 00:00:00 2001
From: dormando <dormando@rydia.net>
Date: Fri, 1 May 2026 13:48:44 -0700
Subject: [PATCH] proto: fix crash in binary protocol

If `watch mutations` is running and a binary protocol SET fails the
logger code attempts to resolve a NULL item reference and will crash.

Reported by Haruto Kimura (Stella)
Comment thread
cwize1 marked this conversation as resolved.
Comment thread
cwize1 marked this conversation as resolved.

Upstream Patch Reference : https://github.com/memcached/memcached/commit/32ea7d8b5b
---
proto_bin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proto_bin.c b/proto_bin.c
index 778ea9a..0e92539 100644
--- a/proto_bin.c
+++ b/proto_bin.c
@@ -1136,7 +1136,7 @@ static void process_bin_update(conn *c, char *extbuf) {
/* FIXME: losing c->cmd since it's translated below. refactor? */
LOGGER_LOG(c->thread->l, LOG_MUTATIONS, LOGGER_ITEM_STORE,
NULL, status, 0, key, nkey, req->message.body.expiration,
- ITEM_clsid(it), c->sfd);
+ 0, c->sfd);

@christopherco Christopher Co (christopherco) Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue (blocking): supply the missing nbytes argument before making this logger path reachable

LOGGER_ITEM_STORE consumes eight variadic fields after entry:

status, comm, key, nkey, nbytes, ttl, clsid, sfd

This call supplies only seven:

status, 0, key, nkey, expiration, 0, c->sfd

Before this patch, evaluating ITEM_clsid(it) crashes because it is NULL. After replacing it with 0, the logger runs and reads one argument past the supplied list. The fields are shifted (expiration becomes nbytes, 0 becomes ttl, and c->sfd becomes clsid), while sfd is read from undefined variadic state. That can corrupt the mutation record and potentially expose stale process data to a watch mutations client.

Please include all eight fields. Based on the successful-store call contract, the failure path should resemble:

Suggested change
+ 0, c->sfd);
NULL, status, 0, key, nkey, vlen + 2,
realtime(req->message.body.expiration), 0, c->sfd);

The same seven-argument mismatch is still present on upstream master, so this should also be reported upstream.

Upstream evidence:

@christopherco Christopher Co (christopherco) Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewing this closer, given that this issue is present in upstream and is a cosmetic logging defect (key and nkey are still correct. nbytes is a log line element, and not part of any memcpy operations), I will change this to non-blocking. It is more important to get this initial crash fixed, and the logging defect is cosmetic.

We should also report this logging issue upstream, and if fixed upstream, backport that fix to our version.


/* Avoid stale data persisting in cache because we failed alloc.
* Unacceptable for SET. Anywhere else too? */
--
2.55.0

6 changes: 5 additions & 1 deletion SPECS/memcached/memcached.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Summary: High Performance, Distributed Memory Object Cache
Name: memcached
Version: 1.6.27
Release: 5%{?dist}
Release: 6%{?dist}
License: BSD
Vendor: Microsoft Corporation
Distribution: Azure Linux
Expand All @@ -19,6 +19,7 @@ Patch1: CVE-2021-43519.patch
Patch2: CVE-2021-44647.patch
Patch3: CVE-2026-24809.patch
Patch4: CVE-2026-47783.patch
Patch5: 0001-proto-fix-crash-in-binary-protocol.patch
BuildRequires: gcc
BuildRequires: libevent-devel
BuildRequires: systemd-devel
Expand Down Expand Up @@ -133,6 +134,9 @@ exit 0
%{_unitdir}/memcached.service

%changelog
* Fri Aug 14 2026 Chris Gunn <chrisgun@microsoft.com> - 1.6.27-6
- Patch: fix crash in binary protocol

* Thu May 21 2026 Azure Linux Security Servicing Account <azurelinux-security@microsoft.com> - 1.6.27-5
- Patch for CVE-2026-47783

Expand Down
Loading