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

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 26 Feb 2026 14:39:48 +0100 Fernando Fernandez Mancera wrote:
> 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.

> 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.

> +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),
-- 
pw-bot: cr