[PATCH 0/2] drbd: fix netlink string policy lengths, raise cpu-mask limit

[PATCH 0/2] drbd: fix netlink string policy lengths, raise cpu-mask limit

[Ionut Nechita]

Patch 1 fixes a regression from the conversion of the DRBD netlink code
away from genl_magic. All NLA_NUL_STRING policies now use the size of
the destination buffer as nla_policy.len, but for NUL strings that field
is the maximum string length excluding the terminating NUL. Every
affected attribute accepts one byte more than fits; nla_strscpy() then
truncates and returns -E2BIG into an unsigned length member, and that
value comes back out as a negative payload length passed to nla_put().
The details are in the patch.

The most realistic way to reach it is a shared secret of exactly
SHARED_SECRET_MAX characters, which is what "openssl rand -hex 32"
produces. Setting one is accepted, and the corruption then happens on
the next privileged status dump rather than at configuration time, so
cause and effect are not obviously related.

Patch 2 raises DRBD_CPU_MASK_SIZE from 32 to 256 so that a cpu-mask
covering more than 112 CPUs can be expressed at all. Userspace already
assumes the larger value.

The order matters: patch 2 on its own would keep the off-by-one and
merely move it from 32 to 256 characters.

For patch 1 I kept the change minimal and restored exactly the bound the
generated code used to emit. Tying each policy to its destination buffer
instead, e.g.

	.len = sizeof_field(struct res_opts, cpu_mask) - 1,

would make the invariant harder to break again, but that is more churn
than a fix during the -rc cycle should carry. I am happy to send it as a
follow-up if you would rather have it that way.

The commit patch 1 fixes first appeared in v7.2-rc1, so there is nothing
to back port to stable.

I found the problem by inspection while looking at the cpu-mask length
limit. There is no user report behind it, and I have not reproduced it
on a live system - I have no DRBD setup here. What I did verify:

  - the boundaries claimed for bitmap_parse(), by extracting the parser
    from lib/bitmap-str.c into a userspace harness: 31 characters
    describe at most 112 CPUs, 255 characters describe 908;
  - that the driver builds on next-20260731, x86_64 defconfig with
    CONFIG_BLK_DEV_DRBD=m, after each patch individually.

One note on patch 2: drbd-utils has carried DRBD_CPU_MASK_SIZE at 256 in
its own copy of these headers since 2023, so this only aligns the kernel
with what userspace already sends. Longer term the ambiguity between the
hex and the list format means a bitmap_parselist() based cpu-mask would
have to be a separate attribute; that is out of scope here.

Based on next-20260731.

Ionut Nechita (2):
  drbd: fix off-by-one in netlink NUL-string policy lengths
  drbd: increase maximum CPU mask size

 drivers/block/drbd/drbd_nl_gen.c | 18 +++++++++---------
 include/uapi/linux/drbd.h        |  7 ++++++-
 2 files changed, 15 insertions(+), 10 deletions(-)

--
2.55.0

[PATCH 1/2] drbd: fix off-by-one in netlink NUL-string policy lengths

[Ionut Nechita]

For NLA_NUL_STRING attributes, nla_policy.len is the maximum length of
the string excluding the terminating NUL, as documented in
include/net/netlink.h. The genl_magic infrastructure encoded that as

	.len = maxlen - (nla_type == NLA_NUL_STRING)

but the hand-written policies that replaced it set .len to the full size
of the destination buffer. Every NUL-string attribute therefore accepts
a string that is one byte too long for the buffer it is copied into.

For such a string validate_nla() succeeds and nla_strscpy() truncates it
and returns -E2BIG. The return value is stored without being checked in
the corresponding unsigned length member, e.g.

	s->cpu_mask_len = nla_strscpy(s->cpu_mask, nla, DRBD_CPU_MASK_SIZE);

set_resource_options() copies the whole struct into resource->res_opts,
so a later dump of the resource options reaches res_opts_to_skb() with
cpu_mask_len set to (__u32)-E2BIG. The payload length handed to
nla_put() is computed there as

	min_t(int, DRBD_CPU_MASK_SIZE,
	      s->cpu_mask_len + (s->cpu_mask_len < DRBD_CPU_MASK_SIZE))

which evaluates to a negative int. __nla_reserve() then stores
nla_attr_size() of that negative value in the u16 nla_len, and
__nla_put() calls memcpy() with an underflowed size argument.

The five net_conf algorithm names, both disk_conf device paths, the
configuration context resource name and the resource cpu-mask are all
affected. The shared secret is the one most likely to be hit in
practice: SHARED_SECRET_MAX is 64, a secret of exactly 64 characters is
what "openssl rand -hex 32" produces, and nla_put_status_info() feeds
the unsanitized net_conf to net_conf_to_skb() for any caller holding
CAP_SYS_ADMIN. The sanitized path clears both the secret and its length,
so an unprivileged status dump is not affected.

Restore the bound the generated code used to emit. With the policy
corrected nla_strscpy() can no longer truncate, so the unchecked return
values become harmless again.

Reaching any of the affected attributes requires CAP_NET_ADMIN. Found by
inspection while looking at the cpu-mask length limit; no user report.

Fixes: 8098eeb693c4 ("drbd: replace genl_magic with explicit netlink serialization")
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
 drivers/block/drbd/drbd_nl_gen.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl_gen.c b/drivers/block/drbd/drbd_nl_gen.c
index fb44b948cec8..5b668e78051b 100644
--- a/drivers/block/drbd/drbd_nl_gen.c
+++ b/drivers/block/drbd/drbd_nl_gen.c
@@ -51,8 +51,8 @@ const struct nla_policy drbd_disconnect_parms_nl_policy[DRBD_A_DISCONNECT_PARMS_
 };
 
 const struct nla_policy drbd_disk_conf_nl_policy[DRBD_A_DISK_CONF_DISABLE_WRITE_SAME + 1] = {
-	[DRBD_A_DISK_CONF_BACKING_DEV] = { .type = NLA_NUL_STRING, .len = 128, },
-	[DRBD_A_DISK_CONF_META_DEV] = { .type = NLA_NUL_STRING, .len = 128, },
+	[DRBD_A_DISK_CONF_BACKING_DEV] = { .type = NLA_NUL_STRING, .len = 128 - 1, },
+	[DRBD_A_DISK_CONF_META_DEV] = { .type = NLA_NUL_STRING, .len = 128 - 1, },
 	[DRBD_A_DISK_CONF_META_DEV_IDX] = { .type = NLA_U32, },
 	[DRBD_A_DISK_CONF_DISK_SIZE] = { .type = NLA_U64, },
 	[DRBD_A_DISK_CONF_MAX_BIO_BVECS] = { .type = NLA_U32, },
@@ -80,17 +80,17 @@ const struct nla_policy drbd_disk_conf_nl_policy[DRBD_A_DISK_CONF_DISABLE_WRITE_
 
 const struct nla_policy drbd_drbd_cfg_context_nl_policy[DRBD_A_DRBD_CFG_CONTEXT_CTX_PEER_ADDR + 1] = {
 	[DRBD_A_DRBD_CFG_CONTEXT_CTX_VOLUME] = { .type = NLA_U32, },
-	[DRBD_A_DRBD_CFG_CONTEXT_CTX_RESOURCE_NAME] = { .type = NLA_NUL_STRING, .len = 128, },
+	[DRBD_A_DRBD_CFG_CONTEXT_CTX_RESOURCE_NAME] = { .type = NLA_NUL_STRING, .len = 128 - 1, },
 	[DRBD_A_DRBD_CFG_CONTEXT_CTX_MY_ADDR] = NLA_POLICY_MAX_LEN(128),
 	[DRBD_A_DRBD_CFG_CONTEXT_CTX_PEER_ADDR] = NLA_POLICY_MAX_LEN(128),
 };
 
 const struct nla_policy drbd_net_conf_nl_policy[DRBD_A_NET_CONF_SOCK_CHECK_TIMEO + 1] = {
-	[DRBD_A_NET_CONF_SHARED_SECRET] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX, },
-	[DRBD_A_NET_CONF_CRAM_HMAC_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX, },
-	[DRBD_A_NET_CONF_INTEGRITY_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX, },
-	[DRBD_A_NET_CONF_VERIFY_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX, },
-	[DRBD_A_NET_CONF_CSUMS_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX, },
+	[DRBD_A_NET_CONF_SHARED_SECRET] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX - 1, },
+	[DRBD_A_NET_CONF_CRAM_HMAC_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX - 1, },
+	[DRBD_A_NET_CONF_INTEGRITY_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX - 1, },
+	[DRBD_A_NET_CONF_VERIFY_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX - 1, },
+	[DRBD_A_NET_CONF_CSUMS_ALG] = { .type = NLA_NUL_STRING, .len = SHARED_SECRET_MAX - 1, },
 	[DRBD_A_NET_CONF_WIRE_PROTOCOL] = { .type = NLA_U32, },
 	[DRBD_A_NET_CONF_CONNECT_INT] = { .type = NLA_U32, },
 	[DRBD_A_NET_CONF_TIMEOUT] = { .type = NLA_U32, },
@@ -143,7 +143,7 @@ const struct nla_policy drbd_peer_device_statistics_nl_policy[DRBD_A_PEER_DEVICE
 };
 
 const struct nla_policy drbd_res_opts_nl_policy[DRBD_A_RES_OPTS_ON_NO_DATA + 1] = {
-	[DRBD_A_RES_OPTS_CPU_MASK] = { .type = NLA_NUL_STRING, .len = DRBD_CPU_MASK_SIZE, },
+	[DRBD_A_RES_OPTS_CPU_MASK] = { .type = NLA_NUL_STRING, .len = DRBD_CPU_MASK_SIZE - 1, },
 	[DRBD_A_RES_OPTS_ON_NO_DATA] = { .type = NLA_U32, },
 };
 
-- 
2.55.0

[PATCH 2/2] drbd: increase maximum CPU mask size

[Ionut Nechita]

The cpu-mask resource option is limited to DRBD_CPU_MASK_SIZE - 1, i.e.
31 characters. bitmap_parse() reads the string as comma separated chunks
of up to eight hex digits, each describing 32 bits, so 31 characters
describe at most 112 CPUs. A full mask cannot be expressed on anything
larger, which by now includes most two-socket machines.

Sparse masks can still reach higher CPU numbers by padding with zero
chunks - "8,0,0,0" selects CPU 99 - but that only helps when few bits
are set.

Userspace has already moved on. drbd-utils carries its own copy of these
headers, where DRBD_CPU_MASK_SIZE has been 256 since 2023 ("drbd:
increase maximum CPU mask size" by Joel Colledge, in LINBIT's
drbd-headers repository). A drbdsetup built against those headers is
rejected with -ERANGE by validate_nla() as soon as the mask does not fit
in the 31 characters this driver accepts.

Raise the limit to match userspace. 255 characters allow a full mask for
908 CPUs: a leading chunk of three hex digits followed by 28 chunks of
eight.

The cpu_mask member of struct res_opts grows accordingly. Two instances
live on the stack, in drbd_adm_resource_opts() and in
drbd_adm_new_resource(), which is uncritical at this size.

This only relaxes an upper bound, so an old drbdsetup keeps working
against a kernel with this change, and a new drbdsetup keeps working
against a kernel without it as long as the mask string still fits in the
old limit.

The new size will likely be too small again eventually. Describing the
mask with bitmap_parselist() would avoid that, but the list and hex
formats are ambiguous for the same input - "8" means CPU 3 in one and
CPU 8 in the other - so it would have to be a separate attribute rather
than a reinterpretation of this one.

Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
 include/uapi/linux/drbd.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/drbd.h b/include/uapi/linux/drbd.h
index cf1ec3eb872f..1327ce1006ac 100644
--- a/include/uapi/linux/drbd.h
+++ b/include/uapi/linux/drbd.h
@@ -368,7 +368,12 @@ enum write_ordering_e {
 #define DRBD_MD_INDEX_FLEX_EXT -2
 #define DRBD_MD_INDEX_FLEX_INT -3
 
-#define DRBD_CPU_MASK_SIZE 32
+/*
+ * Maximum size of the cpu-mask string, including the terminating NUL.
+ * The netlink policy accepts DRBD_CPU_MASK_SIZE - 1 characters, which is
+ * enough for a full mask covering 908 CPUs.
+ */
+#define DRBD_CPU_MASK_SIZE 256
 
 /**
  * struct drbd_genlmsghdr - DRBD specific header used in NETLINK_GENERIC requests
-- 
2.55.0

Re: [PATCH 1/2] drbd: fix off-by-one in netlink NUL-string policy lengths

[Christoph Böhmwalder]

Am 02.08.26 um 13:05 schrieb Ionut Nechita:
> For NLA_NUL_STRING attributes, nla_policy.len is the maximum length of
> the string excluding the terminating NUL, as documented in
> include/net/netlink.h. The genl_magic infrastructure encoded that as
> 
> 	.len = maxlen - (nla_type == NLA_NUL_STRING)
> 
> but the hand-written policies that replaced it set .len to the full size
> of the destination buffer. Every NUL-string attribute therefore accepts
> a string that is one byte too long for the buffer it is copied into.
> 
> For such a string validate_nla() succeeds and nla_strscpy() truncates it
> and returns -E2BIG. The return value is stored without being checked in
> the corresponding unsigned length member, e.g.
> 
> 	s->cpu_mask_len = nla_strscpy(s->cpu_mask, nla, DRBD_CPU_MASK_SIZE);
> 
> set_resource_options() copies the whole struct into resource->res_opts,
> so a later dump of the resource options reaches res_opts_to_skb() with
> cpu_mask_len set to (__u32)-E2BIG. The payload length handed to
> nla_put() is computed there as
> 
> 	min_t(int, DRBD_CPU_MASK_SIZE,
> 	      s->cpu_mask_len + (s->cpu_mask_len < DRBD_CPU_MASK_SIZE))
> 
> which evaluates to a negative int. __nla_reserve() then stores
> nla_attr_size() of that negative value in the u16 nla_len, and
> __nla_put() calls memcpy() with an underflowed size argument.
> 
> The five net_conf algorithm names, both disk_conf device paths, the
> configuration context resource name and the resource cpu-mask are all
> affected. The shared secret is the one most likely to be hit in
> practice: SHARED_SECRET_MAX is 64, a secret of exactly 64 characters is
> what "openssl rand -hex 32" produces, and nla_put_status_info() feeds
> the unsanitized net_conf to net_conf_to_skb() for any caller holding
> CAP_SYS_ADMIN. The sanitized path clears both the secret and its length,
> so an unprivileged status dump is not affected.
> 
> Restore the bound the generated code used to emit. With the policy
> corrected nla_strscpy() can no longer truncate, so the unchecked return
> values become harmless again.
> 
> Reaching any of the affected attributes requires CAP_NET_ADMIN. Found by
> inspection while looking at the cpu-mask length limit; no user report.
> 
> Fixes: 8098eeb693c4 ("drbd: replace genl_magic with explicit netlink serialization")
> Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
> ---
>  drivers/block/drbd/drbd_nl_gen.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Thanks for the patch, good catch. The fix looks correct.

Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>

-- 
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

Re: [PATCH 2/2] drbd: increase maximum CPU mask size

[Christoph Böhmwalder]

Am 02.08.26 um 13:05 schrieb Ionut Nechita:
> The cpu-mask resource option is limited to DRBD_CPU_MASK_SIZE - 1, i.e.
> 31 characters. bitmap_parse() reads the string as comma separated chunks
> of up to eight hex digits, each describing 32 bits, so 31 characters
> describe at most 112 CPUs. A full mask cannot be expressed on anything
> larger, which by now includes most two-socket machines.
> 
> Sparse masks can still reach higher CPU numbers by padding with zero
> chunks - "8,0,0,0" selects CPU 99 - but that only helps when few bits
> are set.
> 
> Userspace has already moved on. drbd-utils carries its own copy of these
> headers, where DRBD_CPU_MASK_SIZE has been 256 since 2023 ("drbd:
> increase maximum CPU mask size" by Joel Colledge, in LINBIT's
> drbd-headers repository). A drbdsetup built against those headers is
> rejected with -ERANGE by validate_nla() as soon as the mask does not fit
> in the 31 characters this driver accepts.
> 
> Raise the limit to match userspace. 255 characters allow a full mask for
> 908 CPUs: a leading chunk of three hex digits followed by 28 chunks of
> eight.
> 
> The cpu_mask member of struct res_opts grows accordingly. Two instances
> live on the stack, in drbd_adm_resource_opts() and in
> drbd_adm_new_resource(), which is uncritical at this size.
> 
> This only relaxes an upper bound, so an old drbdsetup keeps working
> against a kernel with this change, and a new drbdsetup keeps working
> against a kernel without it as long as the mask string still fits in the
> old limit.
> 
> The new size will likely be too small again eventually. Describing the
> mask with bitmap_parselist() would avoid that, but the list and hex
> formats are ambiguous for the same input - "8" means CPU 3 in one and
> CPU 8 in the other - so it would have to be a separate attribute rather
> than a reinterpretation of this one.
> 
> Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
Thanks, this also looks correct.
Just one nit: in the commit message, you reference
drbd_adm_resource_opts and drbd_adm_new_resource; these have been
renamed to drbd_nl_resource_opts_doit and drbd_nl_new_resource_doit,
respectively, in the recent netlink refactoring commit.

But for the code itself:

Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>

-- 
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage