Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct netlink_policy_dump_state.
Additionally update the size of the usage array length before accessing
it. This requires remembering the old size for the memset() and later
assignments.
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: [email protected]
Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by: Kees Cook <[email protected]>
---
net/netlink/policy.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/netlink/policy.c b/net/netlink/policy.c
index 87e3de0fde89..e2f111edf66c 100644
--- a/net/netlink/policy.c
+++ b/net/netlink/policy.c
@@ -21,7 +21,7 @@ struct netlink_policy_dump_state {
struct {
const struct nla_policy *policy;
unsigned int maxtype;
- } policies[];
+ } policies[] __counted_by(n_alloc);
};
static int add_policy(struct netlink_policy_dump_state **statep,
@@ -29,7 +29,7 @@ static int add_policy(struct netlink_policy_dump_state **statep,
unsigned int maxtype)
{
struct netlink_policy_dump_state *state = *statep;
- unsigned int n_alloc, i;
+ unsigned int old_n_alloc, n_alloc, i;
if (!policy || !maxtype)
return 0;
@@ -52,12 +52,13 @@ static int add_policy(struct netlink_policy_dump_state **statep,
if (!state)
return -ENOMEM;
- memset(&state->policies[state->n_alloc], 0,
- flex_array_size(state, policies, n_alloc - state->n_alloc));
-
- state->policies[state->n_alloc].policy = policy;
- state->policies[state->n_alloc].maxtype = maxtype;
+ old_n_alloc = state->n_alloc;
state->n_alloc = n_alloc;
+ memset(&state->policies[old_n_alloc], 0,
+ flex_array_size(state, policies, n_alloc - old_n_alloc));
+
+ state->policies[old_n_alloc].policy = policy;
+ state->policies[old_n_alloc].maxtype = maxtype;
*statep = state;
return 0;
--
2.34.1
On 10/4/23 01:21, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
> array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct netlink_policy_dump_state.
>
> Additionally update the size of the usage array length before accessing
> it. This requires remembering the old size for the memset() and later
> assignments.
>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: Johannes Berg <[email protected]>
> Cc: [email protected]
> Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
> Signed-off-by: Kees Cook <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Thanks
--
Gustavo
> ---
> net/netlink/policy.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/net/netlink/policy.c b/net/netlink/policy.c
> index 87e3de0fde89..e2f111edf66c 100644
> --- a/net/netlink/policy.c
> +++ b/net/netlink/policy.c
> @@ -21,7 +21,7 @@ struct netlink_policy_dump_state {
> struct {
> const struct nla_policy *policy;
> unsigned int maxtype;
> - } policies[];
> + } policies[] __counted_by(n_alloc);
> };
>
> static int add_policy(struct netlink_policy_dump_state **statep,
> @@ -29,7 +29,7 @@ static int add_policy(struct netlink_policy_dump_state **statep,
> unsigned int maxtype)
> {
> struct netlink_policy_dump_state *state = *statep;
> - unsigned int n_alloc, i;
> + unsigned int old_n_alloc, n_alloc, i;
>
> if (!policy || !maxtype)
> return 0;
> @@ -52,12 +52,13 @@ static int add_policy(struct netlink_policy_dump_state **statep,
> if (!state)
> return -ENOMEM;
>
> - memset(&state->policies[state->n_alloc], 0,
> - flex_array_size(state, policies, n_alloc - state->n_alloc));
> -
> - state->policies[state->n_alloc].policy = policy;
> - state->policies[state->n_alloc].maxtype = maxtype;
> + old_n_alloc = state->n_alloc;
> state->n_alloc = n_alloc;
> + memset(&state->policies[old_n_alloc], 0,
> + flex_array_size(state, policies, n_alloc - old_n_alloc));
> +
> + state->policies[old_n_alloc].policy = policy;
> + state->policies[old_n_alloc].maxtype = maxtype;
> *statep = state;
>
> return 0;
Hello:
This patch was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Tue, 3 Oct 2023 16:21:02 -0700 you wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
> array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct netlink_policy_dump_state.
>
> [...]
Here is the summary with links:
- netlink: Annotate struct netlink_policy_dump_state with __counted_by
https://git.kernel.org/netdev/net-next/c/eaede99c3aeb
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html