| Message ID | 20220825024122.1998968-7-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netlink: support reporting missing attributes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 342 this patch: 342 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | success | Errors and warnings before: 5 this patch: 5 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 342 this patch: 342 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Wed, Aug 24, 2022 at 07:41:22PM -0700, Jakub Kicinski wrote: > The actual presence check for the header is in > ethnl_parse_header_dev_get() but it's a few layers in, > and already has a ton of arguments so let's just pick > the low handing fruit and check for missing header in s/handing/hanging/ > the default request handler. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: idosch@nvidia.com Reviewed-by: Ido Schimmel <idosch@nvidia.com> Nice idea. Wanted to ask why you kept the error messages for some of the devlink attributes, but then I figured that user space first needs to learn to interpret 'NLMSGERR_ATTR_MISS_TYPE' and 'NLMSGERR_ATTR_MISS_NEST'. Do you plan to patch iproute2/ethtool after the kernel patches are accepted?
On Thu, 25 Aug 2022 15:19:02 +0300 Ido Schimmel wrote: > Nice idea. Wanted to ask why you kept the error messages for some of the > devlink attributes, but then I figured that user space first needs to > learn to interpret 'NLMSGERR_ATTR_MISS_TYPE' and > 'NLMSGERR_ATTR_MISS_NEST'. Nod. > Do you plan to patch iproute2/ethtool after the kernel patches are accepted? I think ethtool may be doable, I don't remember iproute2 having good support for NLMSGERR_ATTR_OFFS (i.e. resolving offsets), but no, I'm not planning to patch either :( Too few hours in a day :(
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index e26079e11835..0ccb177aaf04 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -361,6 +361,9 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info) ops = ethnl_default_requests[cmd]; if (WARN_ONCE(!ops, "cmd %u has no ethnl_request_ops\n", cmd)) return -EOPNOTSUPP; + if (GENL_REQ_ATTR_CHECK(info, ops->hdr_attr)) + return -EINVAL; + req_info = kzalloc(ops->req_info_size, GFP_KERNEL); if (!req_info) return -ENOMEM;
The actual presence check for the header is in ethnl_parse_header_dev_get() but it's a few layers in, and already has a ton of arguments so let's just pick the low handing fruit and check for missing header in the default request handler. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: idosch@nvidia.com --- net/ethtool/netlink.c | 3 +++ 1 file changed, 3 insertions(+)