netdev - Re: [PATCH net] tools: ynl: c: correct reverse decode of empty attrs

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] [day] [month] [year] [list]

Message-ID: <m2tt9oldmp.fsf@gmail.com>
Date: Fri, 24 Jan 2025 10:17:34 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net,  netdev@...r.kernel.org,  edumazet@...gle.com,
  pabeni@...hat.com,  andrew+netdev@...n.ch,  horms@...nel.org,
  nicolas.dichtel@...nd.com,  willemb@...gle.com
Subject: Re: [PATCH net] tools: ynl: c: correct reverse decode of empty attrs

Jakub Kicinski <kuba@...nel.org> writes:

> netlink reports which attribute was incorrect by sending back
> an attribute offset. Offset points to the address of struct nlattr,
> but to interpret the type we also need the nesting path.
> Attribute IDs have different meaning in different nests
> of the same message.
>
> Correct the condition for "is the offset within current attribute".
> ynl_attr_data_len() does not include the attribute header,
> so the end offset was off by 4 bytes.
>
> This means that we'd always skip over flags and empty nests.
>
> The devmem tests, for example, issues an invalid request with
> empty queue nests, resulting in the following error:
>
>   YNL failed: Kernel error: missing attribute: .queues.ifindex
>
> The message is incorrect, "queues" nest does not have an "ifindex"
> attribute defined. With this fix we decend correctly into the nest:
>
>   YNL failed: Kernel error: missing attribute: .queues.id
>
> Fixes: 86878f14d71a ("tools: ynl: user space helpers")
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> CC: donald.hunter@...il.com
> CC: nicolas.dichtel@...nd.com
> CC: willemb@...gle.com
> ---
>  tools/net/ynl/lib/ynl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
> index e16cef160bc2..ce32cb35007d 100644
> --- a/tools/net/ynl/lib/ynl.c
> +++ b/tools/net/ynl/lib/ynl.c
> @@ -95,7 +95,7 @@ ynl_err_walk(struct ynl_sock *ys, void *start, void *end, unsigned int off,
>  
>  	ynl_attr_for_each_payload(start, data_len, attr) {
>  		astart_off = (char *)attr - (char *)start;
> -		aend_off = astart_off + ynl_attr_data_len(attr);
> +		aend_off = (char *)ynl_attr_data_end(attr) - (char *)start;
>  		if (aend_off <= off)
>  			continue;

Reviewed-by: Donald Hunter <donald.hunter@...il.com>