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

[Donald Hunter <donald.hunter@gmail.com>]

Jakub Kicinski <kuba@kernel.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@kernel.org>
> ---
> CC: donald.hunter@gmail.com
> CC: nicolas.dichtel@6wind.com
> CC: willemb@google.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@gmail.com>