[PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Erick Archer]

The "struct mana_cfg_rx_steer_req_v2" uses a dynamically sized set of
trailing elements. Specifically, it uses a "mana_handle_t" array. So,
use the preferred way in the kernel declaring a flexible array [1].

At the same time, 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).

Also, avoid the open-coded arithmetic in the memory allocator functions
[2] using the "struct_size" macro.

Moreover, use the "offsetof" helper to get the indirect table offset
instead of the "sizeof" operator and avoid the open-coded arithmetic in
pointers using the new flex member. This new structure member also allow
us to remove the "req_indir_tab" variable since it is no longer needed.

Now, it is also possible to use the "flex_array_size" helper to compute
the size of these trailing elements in the "memcpy" function.

Specifically, the first commit adds the flex member and the patches 2 and
3 refactor the consumers of the "struct mana_cfg_rx_steer_req_v2".

This code was detected with the help of Coccinelle, and audited and
modified manually. The Coccinelle script used to detect this code pattern
is the following:

virtual report

@rule1@
type t1;
type t2;
identifier i0;
identifier i1;
identifier i2;
identifier ALLOC =~ "kmalloc|kzalloc|kmalloc_node|kzalloc_node|vmalloc|vzalloc|kvmalloc|kvzalloc";
position p1;
@@

i0 = sizeof(t1) + sizeof(t2) * i1;
...
i2 = ALLOC@p1(..., i0, ...);

@script:python depends on report@
p1 << rule1.p1;
@@

msg = "WARNING: verify allocation on line %s" % (p1[0].line)
coccilib.report.print_report(p1[0],msg)

Link: https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays [1]
Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [2]
Signed-off-by: Erick Archer <erick.archer@outlook.com>
---
Changes in v3:
- Split the changes in various commits to simplify the acceptance process
  (Leon Romanovsky).

Changes in v2:
- Remove the "req_indir_tab" variable (Gustavo A. R. Silva).
- Update the commit message.
- Add the "__counted_by" attribute.

Previous versions:
v1 -> https://lore.kernel.org/linux-hardening/AS8PR02MB7237974EF1B9BAFA618166C38B382@AS8PR02MB7237.eurprd02.prod.outlook.com/
v2 -> https://lore.kernel.org/linux-hardening/AS8PR02MB723729C5A63F24C312FC9CD18B3F2@AS8PR02MB7237.eurprd02.prod.outlook.com/
---
Erick Archer (3):
  net: mana: Add flex array to struct mana_cfg_rx_steer_req_v2
  RDMA/mana_ib: Prefer struct_size over open coded arithmetic
  net: mana: Avoid open coded arithmetic

 drivers/infiniband/hw/mana/qp.c               | 12 +++++-------
 drivers/net/ethernet/microsoft/mana/mana_en.c | 14 ++++++--------
 include/net/mana/mana.h                       |  1 +
 3 files changed, 12 insertions(+), 15 deletions(-)

-- 
2.25.1

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Leon Romanovsky]

On Sat, Apr 06, 2024 at 04:23:34PM +0200, Erick Archer wrote:
> The "struct mana_cfg_rx_steer_req_v2" uses a dynamically sized set of
> trailing elements. Specifically, it uses a "mana_handle_t" array. So,
> use the preferred way in the kernel declaring a flexible array [1].
> 
> At the same time, 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).
> 
> Also, avoid the open-coded arithmetic in the memory allocator functions
> [2] using the "struct_size" macro.
> 
> Moreover, use the "offsetof" helper to get the indirect table offset
> instead of the "sizeof" operator and avoid the open-coded arithmetic in
> pointers using the new flex member. This new structure member also allow
> us to remove the "req_indir_tab" variable since it is no longer needed.
> 
> Now, it is also possible to use the "flex_array_size" helper to compute
> the size of these trailing elements in the "memcpy" function.
> 
> Specifically, the first commit adds the flex member and the patches 2 and
> 3 refactor the consumers of the "struct mana_cfg_rx_steer_req_v2".
> 
> This code was detected with the help of Coccinelle, and audited and
> modified manually. The Coccinelle script used to detect this code pattern
> is the following:
> 
> virtual report
> 
> @rule1@
> type t1;
> type t2;
> identifier i0;
> identifier i1;
> identifier i2;
> identifier ALLOC =~ "kmalloc|kzalloc|kmalloc_node|kzalloc_node|vmalloc|vzalloc|kvmalloc|kvzalloc";
> position p1;
> @@
> 
> i0 = sizeof(t1) + sizeof(t2) * i1;
> ...
> i2 = ALLOC@p1(..., i0, ...);
> 
> @script:python depends on report@
> p1 << rule1.p1;
> @@
> 
> msg = "WARNING: verify allocation on line %s" % (p1[0].line)
> coccilib.report.print_report(p1[0],msg)
> 
> Link: https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays [1]
> Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [2]
> Signed-off-by: Erick Archer <erick.archer@outlook.com>
> ---
> Changes in v3:
> - Split the changes in various commits to simplify the acceptance process
>   (Leon Romanovsky).
> 
> Changes in v2:
> - Remove the "req_indir_tab" variable (Gustavo A. R. Silva).
> - Update the commit message.
> - Add the "__counted_by" attribute.
> 
> Previous versions:
> v1 -> https://lore.kernel.org/linux-hardening/AS8PR02MB7237974EF1B9BAFA618166C38B382@AS8PR02MB7237.eurprd02.prod.outlook.com/
> v2 -> https://lore.kernel.org/linux-hardening/AS8PR02MB723729C5A63F24C312FC9CD18B3F2@AS8PR02MB7237.eurprd02.prod.outlook.com/
> ---
> Erick Archer (3):
>   net: mana: Add flex array to struct mana_cfg_rx_steer_req_v2
>   RDMA/mana_ib: Prefer struct_size over open coded arithmetic
>   net: mana: Avoid open coded arithmetic

Unfortunately, I still can't take RDMA patch alone without the netdev
patches.

Jakub, do you want shared branch for this series or should I take
everything through RDMA tree as netdev part is small enough?

Thanks

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Jakub Kicinski]

On Mon, 8 Apr 2024 14:07:30 +0300 Leon Romanovsky wrote:
> Jakub, do you want shared branch for this series or should I take
> everything through RDMA tree as netdev part is small enough?

Shared branch would be good. Ed has some outstanding patches 
to refactor the ethtool RSS API.

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Leon Romanovsky]

On Mon, Apr 08, 2024 at 06:36:57PM -0700, Jakub Kicinski wrote:
> On Mon, 8 Apr 2024 14:07:30 +0300 Leon Romanovsky wrote:
> > Jakub, do you want shared branch for this series or should I take
> > everything through RDMA tree as netdev part is small enough?
> 
> Shared branch would be good. Ed has some outstanding patches 
> to refactor the ethtool RSS API.

Great, I'll wait for a day or two to give people time to review and
then I'll create a shared branch.

Thanks

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Edward Cree]

On 09/04/2024 02:36, Jakub Kicinski wrote:
> On Mon, 8 Apr 2024 14:07:30 +0300 Leon Romanovsky wrote:
>> Jakub, do you want shared branch for this series or should I take
>> everything through RDMA tree as netdev part is small enough?
> 
> Shared branch would be good. Ed has some outstanding patches 
> to refactor the ethtool RSS API.

For the record I am extremely unlikely to have time to get those
 done this cycle :(
Though in any case fwiw it doesn't look like this series touches
 anything that would conflict; mana doesn't appear to support
 custom RSS contexts and besides the changes are well away from
 the ethtool API handling.

-e

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Jakub Kicinski]

On Tue, 9 Apr 2024 18:01:40 +0100 Edward Cree wrote:
> > Shared branch would be good. Ed has some outstanding patches 
> > to refactor the ethtool RSS API.  
> 
> For the record I am extremely unlikely to have time to get those
>  done this cycle :(
> Though in any case fwiw it doesn't look like this series touches
>  anything that would conflict; mana doesn't appear to support
>  custom RSS contexts and besides the changes are well away from
>  the ethtool API handling.

Better safe than sorry, since the change applies cleanly on an -rc tag
having it applied to both trees should be very little extra work.

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Leon Romanovsky]

On Tue, Apr 09, 2024 at 02:44:19PM -0700, Jakub Kicinski wrote:
> On Tue, 9 Apr 2024 18:01:40 +0100 Edward Cree wrote:
> > > Shared branch would be good. Ed has some outstanding patches 
> > > to refactor the ethtool RSS API.  
> > 
> > For the record I am extremely unlikely to have time to get those
> >  done this cycle :(
> > Though in any case fwiw it doesn't look like this series touches
> >  anything that would conflict; mana doesn't appear to support
> >  custom RSS contexts and besides the changes are well away from
> >  the ethtool API handling.
> 
> Better safe than sorry, since the change applies cleanly on an -rc tag
> having it applied to both trees should be very little extra work.

I prepared mana-ib-flex branch https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=mana-ib-flex
and merge ti to our wip branch https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=e537deecda03e0911e9406095ccd48bd42f328c7

Thanks

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[Jakub Kicinski]

On Thu, 11 Apr 2024 13:58:39 +0300 Leon Romanovsky wrote:
> I prepared mana-ib-flex branch https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=mana-ib-flex
> and merge ti to our wip branch https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=e537deecda03e0911e9406095ccd48bd42f328c7

Thanks!

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Leon Romanovsky <leon@kernel.org>:

On Sat,  6 Apr 2024 16:23:34 +0200 you wrote:
> The "struct mana_cfg_rx_steer_req_v2" uses a dynamically sized set of
> trailing elements. Specifically, it uses a "mana_handle_t" array. So,
> use the preferred way in the kernel declaring a flexible array [1].
> 
> At the same time, 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).
> 
> [...]

Here is the summary with links:
  - [v3,1/3] net: mana: Add flex array to struct mana_cfg_rx_steer_req_v2
    https://git.kernel.org/netdev/net-next/c/bfec4e18f943
  - [v3,2/3] RDMA/mana_ib: Prefer struct_size over open coded arithmetic
    https://git.kernel.org/netdev/net-next/c/29b8e13a8b4c
  - [v3,3/3] net: mana: Avoid open coded arithmetic
    https://git.kernel.org/netdev/net-next/c/a68292eb4316

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html