* [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation
@ 2023-01-07 17:10 Ido Schimmel
2023-01-10 1:08 ` Alexander H Duyck
2023-01-10 3:50 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Ido Schimmel @ 2023-01-07 17:10 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, jhs, xiyou.wangcong, jiri,
willemb, simon.horman, john.hurley, harperchen1110, johannes,
Ido Schimmel
The 'TCA_MPLS_LABEL' attribute is of 'NLA_U32' type, but has a
validation type of 'NLA_VALIDATE_FUNCTION'. This is an invalid
combination according to the comment above 'struct nla_policy':
"
Meaning of `validate' field, use via NLA_POLICY_VALIDATE_FN:
NLA_BINARY Validation function called for the attribute.
All other Unused - but note that it's a union
"
This can trigger the warning [1] in nla_get_range_unsigned() when
validation of the attribute fails. Despite being of 'NLA_U32' type, the
associated 'min'/'max' fields in the policy are negative as they are
aliased by the 'validate' field.
Fix by changing the attribute type to 'NLA_BINARY' which is consistent
with the above comment and all other users of NLA_POLICY_VALIDATE_FN().
As a result, move the length validation to the validation function.
No regressions in MPLS tests:
# ./tdc.py -f tc-tests/actions/mpls.json
[...]
# echo $?
0
[1]
WARNING: CPU: 0 PID: 17743 at lib/nlattr.c:118
nla_get_range_unsigned+0x1d8/0x1e0 lib/nlattr.c:117
Modules linked in:
CPU: 0 PID: 17743 Comm: syz-executor.0 Not tainted 6.1.0-rc8 #3
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
RIP: 0010:nla_get_range_unsigned+0x1d8/0x1e0 lib/nlattr.c:117
[...]
Call Trace:
<TASK>
__netlink_policy_dump_write_attr+0x23d/0x990 net/netlink/policy.c:310
netlink_policy_dump_write_attr+0x22/0x30 net/netlink/policy.c:411
netlink_ack_tlv_fill net/netlink/af_netlink.c:2454 [inline]
netlink_ack+0x546/0x760 net/netlink/af_netlink.c:2506
netlink_rcv_skb+0x1b7/0x240 net/netlink/af_netlink.c:2546
rtnetlink_rcv+0x18/0x20 net/core/rtnetlink.c:6109
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x5e9/0x6b0 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x739/0x860 net/netlink/af_netlink.c:1921
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg net/socket.c:734 [inline]
____sys_sendmsg+0x38f/0x500 net/socket.c:2482
___sys_sendmsg net/socket.c:2536 [inline]
__sys_sendmsg+0x197/0x230 net/socket.c:2565
__do_sys_sendmsg net/socket.c:2574 [inline]
__se_sys_sendmsg net/socket.c:2572 [inline]
__x64_sys_sendmsg+0x42/0x50 net/socket.c:2572
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Link: https://lore.kernel.org/netdev/CAO4mrfdmjvRUNbDyP0R03_DrD_eFCLCguz6OxZ2TYRSv0K9gxA@mail.gmail.com/
Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
Reported-by: Wei Chen <harperchen1110@gmail.com>
Tested-by: Wei Chen <harperchen1110@gmail.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/sched/act_mpls.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
index ff47ce4d3968..6b26bdb999d7 100644
--- a/net/sched/act_mpls.c
+++ b/net/sched/act_mpls.c
@@ -134,6 +134,11 @@ static int valid_label(const struct nlattr *attr,
{
const u32 *label = nla_data(attr);
+ if (nla_len(attr) != sizeof(*label)) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid MPLS label length");
+ return -EINVAL;
+ }
+
if (*label & ~MPLS_LABEL_MASK || *label == MPLS_LABEL_IMPLNULL) {
NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
return -EINVAL;
@@ -145,7 +150,8 @@ static int valid_label(const struct nlattr *attr,
static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
[TCA_MPLS_PARMS] = NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
[TCA_MPLS_PROTO] = { .type = NLA_U16 },
- [TCA_MPLS_LABEL] = NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
+ [TCA_MPLS_LABEL] = NLA_POLICY_VALIDATE_FN(NLA_BINARY,
+ valid_label),
[TCA_MPLS_TC] = NLA_POLICY_RANGE(NLA_U8, 0, 7),
[TCA_MPLS_TTL] = NLA_POLICY_MIN(NLA_U8, 1),
[TCA_MPLS_BOS] = NLA_POLICY_RANGE(NLA_U8, 0, 1),
--
2.37.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation
2023-01-07 17:10 [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation Ido Schimmel
@ 2023-01-10 1:08 ` Alexander H Duyck
2023-01-10 3:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Alexander H Duyck @ 2023-01-10 1:08 UTC (permalink / raw)
To: Ido Schimmel, netdev
Cc: davem, kuba, pabeni, edumazet, jhs, xiyou.wangcong, jiri,
willemb, simon.horman, john.hurley, harperchen1110, johannes
On Sat, 2023-01-07 at 19:10 +0200, Ido Schimmel wrote:
> The 'TCA_MPLS_LABEL' attribute is of 'NLA_U32' type, but has a
> validation type of 'NLA_VALIDATE_FUNCTION'. This is an invalid
> combination according to the comment above 'struct nla_policy':
>
> "
> Meaning of `validate' field, use via NLA_POLICY_VALIDATE_FN:
> NLA_BINARY Validation function called for the attribute.
> All other Unused - but note that it's a union
> "
>
> This can trigger the warning [1] in nla_get_range_unsigned() when
> validation of the attribute fails. Despite being of 'NLA_U32' type, the
> associated 'min'/'max' fields in the policy are negative as they are
> aliased by the 'validate' field.
>
> Fix by changing the attribute type to 'NLA_BINARY' which is consistent
> with the above comment and all other users of NLA_POLICY_VALIDATE_FN().
> As a result, move the length validation to the validation function.
>
> No regressions in MPLS tests:
>
> # ./tdc.py -f tc-tests/actions/mpls.json
> [...]
> # echo $?
> 0
>
> [1]
> WARNING: CPU: 0 PID: 17743 at lib/nlattr.c:118
> nla_get_range_unsigned+0x1d8/0x1e0 lib/nlattr.c:117
> Modules linked in:
> CPU: 0 PID: 17743 Comm: syz-executor.0 Not tainted 6.1.0-rc8 #3
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
> RIP: 0010:nla_get_range_unsigned+0x1d8/0x1e0 lib/nlattr.c:117
> [...]
> Call Trace:
> <TASK>
> __netlink_policy_dump_write_attr+0x23d/0x990 net/netlink/policy.c:310
> netlink_policy_dump_write_attr+0x22/0x30 net/netlink/policy.c:411
> netlink_ack_tlv_fill net/netlink/af_netlink.c:2454 [inline]
> netlink_ack+0x546/0x760 net/netlink/af_netlink.c:2506
> netlink_rcv_skb+0x1b7/0x240 net/netlink/af_netlink.c:2546
> rtnetlink_rcv+0x18/0x20 net/core/rtnetlink.c:6109
> netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
> netlink_unicast+0x5e9/0x6b0 net/netlink/af_netlink.c:1345
> netlink_sendmsg+0x739/0x860 net/netlink/af_netlink.c:1921
> sock_sendmsg_nosec net/socket.c:714 [inline]
> sock_sendmsg net/socket.c:734 [inline]
> ____sys_sendmsg+0x38f/0x500 net/socket.c:2482
> ___sys_sendmsg net/socket.c:2536 [inline]
> __sys_sendmsg+0x197/0x230 net/socket.c:2565
> __do_sys_sendmsg net/socket.c:2574 [inline]
> __se_sys_sendmsg net/socket.c:2572 [inline]
> __x64_sys_sendmsg+0x42/0x50 net/socket.c:2572
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> Link: https://lore.kernel.org/netdev/CAO4mrfdmjvRUNbDyP0R03_DrD_eFCLCguz6OxZ2TYRSv0K9gxA@mail.gmail.com/
> Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> Reported-by: Wei Chen <harperchen1110@gmail.com>
> Tested-by: Wei Chen <harperchen1110@gmail.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
> net/sched/act_mpls.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> index ff47ce4d3968..6b26bdb999d7 100644
> --- a/net/sched/act_mpls.c
> +++ b/net/sched/act_mpls.c
> @@ -134,6 +134,11 @@ static int valid_label(const struct nlattr *attr,
> {
> const u32 *label = nla_data(attr);
>
> + if (nla_len(attr) != sizeof(*label)) {
> + NL_SET_ERR_MSG_MOD(extack, "Invalid MPLS label length");
> + return -EINVAL;
> + }
> +
> if (*label & ~MPLS_LABEL_MASK || *label == MPLS_LABEL_IMPLNULL) {
> NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
> return -EINVAL;
> @@ -145,7 +150,8 @@ static int valid_label(const struct nlattr *attr,
> static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
> [TCA_MPLS_PARMS] = NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
> [TCA_MPLS_PROTO] = { .type = NLA_U16 },
> - [TCA_MPLS_LABEL] = NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
> + [TCA_MPLS_LABEL] = NLA_POLICY_VALIDATE_FN(NLA_BINARY,
> + valid_label),
> [TCA_MPLS_TC] = NLA_POLICY_RANGE(NLA_U8, 0, 7),
> [TCA_MPLS_TTL] = NLA_POLICY_MIN(NLA_U8, 1),
> [TCA_MPLS_BOS] = NLA_POLICY_RANGE(NLA_U8, 0, 1),
Looks good to me.
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation
2023-01-07 17:10 [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation Ido Schimmel
2023-01-10 1:08 ` Alexander H Duyck
@ 2023-01-10 3:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-10 3:50 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, jhs, xiyou.wangcong, jiri,
willemb, simon.horman, john.hurley, harperchen1110, johannes
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 7 Jan 2023 19:10:04 +0200 you wrote:
> The 'TCA_MPLS_LABEL' attribute is of 'NLA_U32' type, but has a
> validation type of 'NLA_VALIDATE_FUNCTION'. This is an invalid
> combination according to the comment above 'struct nla_policy':
>
> "
> Meaning of `validate' field, use via NLA_POLICY_VALIDATE_FN:
> NLA_BINARY Validation function called for the attribute.
> All other Unused - but note that it's a union
> "
>
> [...]
Here is the summary with links:
- [net] net/sched: act_mpls: Fix warning during failed attribute validation
https://git.kernel.org/netdev/net/c/9e17f99220d1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-10 3:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-07 17:10 [PATCH net] net/sched: act_mpls: Fix warning during failed attribute validation Ido Schimmel
2023-01-10 1:08 ` Alexander H Duyck
2023-01-10 3:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).