The dcbnl_bcn_setcfg uses erroneous policy to parse tb[DCB_ATTR_BCN],
which is introduced in commit 859ee3c43812 ("DCB: Add support for DCB
BCN"). Please see the comment in below code
static int dcbnl_bcn_setcfg(...)
{
...
ret = nla_parse_nested_deprecated(..., dcbnl_pfc_up_nest, .. )
// !!! dcbnl_pfc_up_nest for attributes
// DCB_PFC_UP_ATTR_0 to DCB_PFC_UP_ATTR_ALL in enum dcbnl_pfc_up_attrs
...
for (i = DCB_BCN_ATTR_RP_0; i <= DCB_BCN_ATTR_RP_7; i++) {
// !!! DCB_BCN_ATTR_RP_0 to DCB_BCN_ATTR_RP_7 in enum dcbnl_bcn_attrs
...
value_byte = nla_get_u8(data[i]);
...
}
...
for (i = DCB_BCN_ATTR_BCNA_0; i <= DCB_BCN_ATTR_RI; i++) {
// !!! DCB_BCN_ATTR_BCNA_0 to DCB_BCN_ATTR_RI in enum dcbnl_bcn_attrs
...
value_int = nla_get_u32(data[i]);
...
}
...
}
That is, the nla_parse_nested_deprecated uses dcbnl_pfc_up_nest
attributes to parse nlattr defined in dcbnl_pfc_up_attrs. But the
following access code fetch each nlattr as dcbnl_bcn_attrs attributes.
By looking up the associated nla_policy for dcbnl_bcn_attrs. We can find
the beginning part of these two policies are "same".
static const struct nla_policy dcbnl_pfc_up_nest[...] = {
[DCB_PFC_UP_ATTR_0] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_1] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_2] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_3] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_4] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_5] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_6] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_7] = {.type = NLA_U8},
[DCB_PFC_UP_ATTR_ALL] = {.type = NLA_FLAG},
};
static const struct nla_policy dcbnl_bcn_nest[...] = {
[DCB_BCN_ATTR_RP_0] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_1] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_2] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_3] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_4] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_5] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_6] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_7] = {.type = NLA_U8},
[DCB_BCN_ATTR_RP_ALL] = {.type = NLA_FLAG},
// from here is somewhat different
[DCB_BCN_ATTR_BCNA_0] = {.type = NLA_U32},
...
[DCB_BCN_ATTR_ALL] = {.type = NLA_FLAG},
};
Therefore, the current code is buggy and this
nla_parse_nested_deprecated could overflow the dcbnl_pfc_up_nest and use
the adjacent nla_policy to parse attributes from DCB_BCN_ATTR_BCNA_0.
This patch use correct dcbnl_bcn_nest policy to parse the
tb[DCB_ATTR_BCN] nested TLV.
Fixes: 859ee3c43812 ("DCB: Add support for DCB BCN")
Signed-off-by: Lin Ma <[email protected]>
---
net/dcb/dcbnl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index c0c438128575..2e6b8c8fd2de 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -980,7 +980,7 @@ static int dcbnl_bcn_setcfg(struct net_device *netdev, struct nlmsghdr *nlh,
return -EOPNOTSUPP;
ret = nla_parse_nested_deprecated(data, DCB_BCN_ATTR_MAX,
- tb[DCB_ATTR_BCN], dcbnl_pfc_up_nest,
+ tb[DCB_ATTR_BCN], dcbnl_bcn_nest,
NULL);
if (ret)
return ret;
--
2.17.1
On Mon, Jul 31, 2023 at 12:52:16PM +0800, Lin Ma wrote:
> The dcbnl_bcn_setcfg uses erroneous policy to parse tb[DCB_ATTR_BCN],
> which is introduced in commit 859ee3c43812 ("DCB: Add support for DCB
> BCN"). Please see the comment in below code
>
> static int dcbnl_bcn_setcfg(...)
> {
> ...
> ret = nla_parse_nested_deprecated(..., dcbnl_pfc_up_nest, .. )
> // !!! dcbnl_pfc_up_nest for attributes
> // DCB_PFC_UP_ATTR_0 to DCB_PFC_UP_ATTR_ALL in enum dcbnl_pfc_up_attrs
> ...
> for (i = DCB_BCN_ATTR_RP_0; i <= DCB_BCN_ATTR_RP_7; i++) {
> // !!! DCB_BCN_ATTR_RP_0 to DCB_BCN_ATTR_RP_7 in enum dcbnl_bcn_attrs
> ...
> value_byte = nla_get_u8(data[i]);
> ...
> }
> ...
> for (i = DCB_BCN_ATTR_BCNA_0; i <= DCB_BCN_ATTR_RI; i++) {
> // !!! DCB_BCN_ATTR_BCNA_0 to DCB_BCN_ATTR_RI in enum dcbnl_bcn_attrs
> ...
> value_int = nla_get_u32(data[i]);
> ...
> }
> ...
> }
>
> That is, the nla_parse_nested_deprecated uses dcbnl_pfc_up_nest
> attributes to parse nlattr defined in dcbnl_pfc_up_attrs. But the
> following access code fetch each nlattr as dcbnl_bcn_attrs attributes.
> By looking up the associated nla_policy for dcbnl_bcn_attrs. We can find
> the beginning part of these two policies are "same".
>
> static const struct nla_policy dcbnl_pfc_up_nest[...] = {
> [DCB_PFC_UP_ATTR_0] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_1] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_2] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_3] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_4] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_5] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_6] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_7] = {.type = NLA_U8},
> [DCB_PFC_UP_ATTR_ALL] = {.type = NLA_FLAG},
> };
>
> static const struct nla_policy dcbnl_bcn_nest[...] = {
> [DCB_BCN_ATTR_RP_0] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_1] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_2] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_3] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_4] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_5] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_6] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_7] = {.type = NLA_U8},
> [DCB_BCN_ATTR_RP_ALL] = {.type = NLA_FLAG},
> // from here is somewhat different
> [DCB_BCN_ATTR_BCNA_0] = {.type = NLA_U32},
> ...
> [DCB_BCN_ATTR_ALL] = {.type = NLA_FLAG},
> };
>
> Therefore, the current code is buggy and this
> nla_parse_nested_deprecated could overflow the dcbnl_pfc_up_nest and use
> the adjacent nla_policy to parse attributes from DCB_BCN_ATTR_BCNA_0.
>
> This patch use correct dcbnl_bcn_nest policy to parse the
> tb[DCB_ATTR_BCN] nested TLV.
>
> Fixes: 859ee3c43812 ("DCB: Add support for DCB BCN")
> Signed-off-by: Lin Ma <[email protected]>
Reviewed-by: Simon Horman <[email protected]>