netdev - Re: [PATCH net] openvswitch: Fix unsafe attribute parsing in output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50b641a0-50d3-448c-a6f3-b4e143e87c08@ovn.org>
Date: Tue, 6 May 2025 17:24:56 +0200
From: Ilya Maximets <i.maximets@....org>
To: Eelco Chaudron <echaudro@...hat.com>, netdev@...r.kernel.org
Cc: i.maximets@....org, dev@...nvswitch.org, aconole@...hat.com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org
Subject: Re: [PATCH net] openvswitch: Fix unsafe attribute parsing in
 output_userspace()

On 5/6/25 4:28 PM, Eelco Chaudron wrote:
> This patch replaces the manual Netlink attribute iteration in
> output_userspace() with nla_for_each_nested(), which ensures that only
> well-formed attributes are processed.
> 
> Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
> Signed-off-by: Eelco Chaudron <echaudro@...hat.com>
> ---
>  net/openvswitch/actions.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Should probably add a Reported-by tag referencing the report id
and mention the reporter in the commit message.

Otherwise, LGTM.  Thanks!

Acked-by: Ilya Maximets <i.maximets@....org>

> 
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 61fea7baae5d..2f22ca59586f 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -975,8 +975,7 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb,
>  	upcall.cmd = OVS_PACKET_CMD_ACTION;
>  	upcall.mru = OVS_CB(skb)->mru;
>  
> -	for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> -	     a = nla_next(a, &rem)) {
> +	nla_for_each_nested(a, attr, rem) {
>  		switch (nla_type(a)) {
>  		case OVS_USERSPACE_ATTR_USERDATA:
>  			upcall.userdata = a;