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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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.
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
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
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.
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
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!
Hello:
This series was applied to netdev/net-next.git (main)
by Leon Romanovsky <[email protected]>:
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