netdev - Re: [PATCH] qed/red_ll2: Fix undefined behavior bug in struct qed_ll2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 23 Sep 2023 12:43:59 -0700
From: Kees Cook <kees@...nel.org>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Ariel Elior <aelior@...vell.com>, Manish Chopra <manishc@...vell.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Michal Kalderon <Michal.Kalderon@...ium.com>
CC: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH] qed/red_ll2: Fix undefined behavior bug in struct qed_ll2_info

On September 23, 2023 6:15:59 PM PDT, "Gustavo A. R. Silva" <gustavoars@...nel.org> 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@...r.kernel.org
>Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>

Reviewed-by: Kees Cook <keescook@...omium.org>

-- 
Kees Cook