Re: [PATCH net-next 1/4] netlink: truncate overlength attribute list in nla_nest_end()

[David Ahern <dsahern@gmail.com>]

On 1/23/21 2:03 PM, Edwin Peer wrote:
> On Sat, Jan 23, 2021 at 12:42 PM Edwin Peer <edwin.peer@broadcom.com> wrote:
> 
>> Then, if nla_put() can detect nesting errors, there's the issue of
>> what to do in the case of errors. Case in point, the IFLA_VFINFO_LIST
>> scenario would now require explicit error handling in the generator
>> logic, because we can't fail hard at that point. We would need to be
>> sure we propagate all possible nesting errors up to a common location
>> (probably where the nest ends, which is where we're dealing with the
>> problem in this solution), set the truncated flag and carry on (for
>> the same net effect the trim in nla_nest_end() has).
> 
> Also, the unwind here turns out to be just as complicated, requiring a
> traversal from the start and a trim anyway, because the attribute that
> triggers the overflow may be deep inside the hierarchy. We can't
> simply truncate at this point. We should remove whole elements at the
> uppermost level, or risk having partially filled attribute trees
> rather than missing attributes at the level of the exceeded nest -
> which may be worse.
> 

I'm not a fan of the skb trim idea. I think it would be better to figure
out how to stop adding to the skb when an attr length is going to exceed
64kB. Not failing hard with an error (ip link sh needs to succeed), but
truncating the specific attribute of a message with a flag so userspace
knows it is short.