Re: [PATCH] qed/red_ll2: Fix undefined behavior bug in struct qed_ll2_info

[Kees Cook]

On September 23, 2023 6:15:59 PM PDT, "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx> wrote:
>The flexible structure (a structure that contains a flexible-array member
>at the end) `qed_ll2_tx_packet` is nested within the second layer of
>`struct qed_ll2_info`:
>
>struct qed_ll2_tx_packet {
>	...
>        /* Flexible Array of bds_set determined by max_bds_per_packet */
>        struct {
>                struct core_tx_bd *txq_bd;
>                dma_addr_t tx_frag;
>                u16 frag_len;
>        } bds_set[];
>};
>
>struct qed_ll2_tx_queue {
>	...
>	struct qed_ll2_tx_packet cur_completing_packet;
>};
>
>struct qed_ll2_info {
>	...
>	struct qed_ll2_tx_queue tx_queue;
>        struct qed_ll2_cbs cbs;
>};

Nice find! Was this located with -Wflex-array-member-not-at-end ? 

> [...]
>Fix this by moving the declaration of `cbs` to the  middle of its
>containing structure `qed_ll2_info`, preventing it from being
>overwritten by the contents of `bds_set` at run-time.
>
>This bug was introduced in 2017, when `bds_set` was converted to a
>one-element array, and started to be used as a Variable Length Object
>(VLO) at run-time.
>
>Fixes: f5823fe6897c ("qed: Add ll2 option to limit the number of bds per packet")
>Cc: stable@xxxxxxxxxxxxxxx
>Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>


-- 
Kees Cook