On 10/7/18 4:36 AM, Christian Brauner wrote:
>> + if (cb->strict_check) {
>> + struct ifinfomsg *ifm;
>>
>> - extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
>> - IFLA_EXT_MASK);
>> - if (extfilt) {
>> - if (nla_len(extfilt) < sizeof(filter_mask))
>> - return -EINVAL;
>> + if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
>> + NL_SET_ERR_MSG(extack, "Invalid header");
>> + return -EINVAL;
>> + }
>> +
>> + ifm = nlmsg_data(nlh);
>> + if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
>> + ifm->ifi_change || ifm->ifi_index) {
>> + NL_SET_ERR_MSG(extack, "Invalid values in header for
>> dump request");
>> + return -EINVAL;
>> + }
>> + }
>>
>> - filter_mask = nla_get_u32(extfilt);
>> + err = nlmsg_parse(nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
>> + ifla_policy, extack);
>> + if (err < 0) {
>> + if (cb->strict_check)
>> + return -EINVAL;
>> + goto walk_entries;
>> + }
>
> What's the point of moving this out of the
> if (cb->strict_check) {} branch above? This looks like it would cause
> the same parse warnings that we're trying to get rid of in inet{4,6}
> dumps.
Link messages don't have the problem in general because they use
ifinfomsg as the header - which is the one abused for other message
types. That said ...
> Seems to make more sense to make the nlmsg_parse() itself conditional as
> well unless I'm lacking context.
... I now have nlmsg_parse and nlmsg_parse_strict.
