Re: [PATCH 1/2 net-next v2] ipv4: validate IPV4_DEVCONF attributes properly

On 2/28/26 7:43 PM, Jakub Kicinski wrote:

On Thu, 26 Feb 2026 14:39:48 +0100 Fernando Fernandez Mancera wrote:

quoted

As the IPV4_DEVCONF netlink attributes are not being validated, it is
possible to use netlink to set read-only values like mc_forwarding. In
addition, valid ranges are not being validated neither but that is less
relevant as they aren't in sysctl.

To avoid similar situations in the future, define a NLA policy for
IPV4_DEVCONF attributes which are nested in IFLA_INET_CONF.

Very nice, I think we should drop the Fixes tag tho.
Adding missed validation is always tricky, we don't really want people
to backport this to stable releases, the risk of regression (of broken
user space) is too high. Unless there's some crash this prevents, in
which case we'd need a more targeted fix for just those values in net.

quoted

Please note that MEDIUM_ID is defined as NLA_U32 too because currently
its usage through netlink is broken for its valid value -1. Modifying
the type to NLA_S32 would break existing users of set/get netlink
operation.

Say more? The policy type not matching the accessor used by the kernel
is probably fine in this case (since there's a common accessor used for
all attrs). If it helps the policy, we can use a different type.

The problem is not only not matching the accessor.. the problem is that 
while it was not validated if users were using NLA_U32 as indicated by 
the original implementation (see blamed commit), this would break them.

Is it one option to set the type to NLA_S32 and wait to see if someone 
complains? I am not sure how many people might be using it considering 
the type is wrong.

Thanks,
Fernando.

quoted

+static const struct nla_policy inet_devconf_policy[IPV4_DEVCONF_MAX + 1] = {
+	[IPV4_DEVCONF_FORWARDING]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_MC_FORWARDING]			  = { .type = NLA_REJECT },
+	[IPV4_DEVCONF_PROXY_ARP]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_ACCEPT_REDIRECTS]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_SECURE_REDIRECTS]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_SEND_REDIRECTS]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_SHARED_MEDIA]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_RP_FILTER]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 2),
+	[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE]		  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_BOOTP_RELAY]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_LOG_MARTIANS]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_TAG]				  = { .type = NLA_U32 },
+	[IPV4_DEVCONF_ARPFILTER]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_MEDIUM_ID]			  = { .type = NLA_U32 },
+	[IPV4_DEVCONF_NOXFRM]				  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_NOPOLICY]				  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_FORCE_IGMP_VERSION]		  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 3),
+	[IPV4_DEVCONF_ARP_ANNOUNCE]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 2),
+	[IPV4_DEVCONF_ARP_IGNORE]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 8),
+	[IPV4_DEVCONF_PROMOTE_SECONDARIES]		  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_ARP_ACCEPT]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_ARP_NOTIFY]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_ACCEPT_LOCAL]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_SRC_VMARK]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_PROXY_ARP_PVLAN]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_ROUTE_LOCALNET]			  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),
+	[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL] = { .type = NLA_U32 },
+	[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL] = { .type = NLA_U32 },
+	[IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN]	  = NLA_POLICY_RANGE(NLA_U32,
+									     0, 1),

The indentation is rather awkward, please adjust to fit the common case
on one line and special case the long ones.

	// mis-adjust when needed
	[IPV4_DEVCONF_PROMOTE_SECONDARIES] = NLA_POLICY_RANGE(NLA_U32, 0, 1),
	// common / normal case
	[IPV4_DEVCONF_ARP_ACCEPT]	= NLA_POLICY_RANGE(NLA_U32, 0, 1),
	[IPV4_DEVCONF_ARP_NOTIFY]	= NLA_POLICY_RANGE(NLA_U32, 0, 1),
	[IPV4_DEVCONF_ACCEPT_LOCAL]	= NLA_POLICY_RANGE(NLA_U32, 0, 1),
	...
	// overflow type fully to next line if doesn't fit even mis-adjusted
	[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL] =
		{ .type = NLA_U32 },
	[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL] =
		{ .type = NLA_U32 },
	[IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN] =
		NLA_POLICY_RANGE(NLA_U32, 0, 1),

Thanks for the suggestion!