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

[Ionut Nechita <ionut.nechita@windriver.com>]

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