linux-hardening - [PATCH 0/2] can: treewide: decorate flexible array members with __counted

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-next>] [day] [month] [year] [list]

Message-Id: <20240609045419.240265-1-mailhol.vincent@wanadoo.fr>
Date: Sun,  9 Jun 2024 13:54:17 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Marc Kleine-Budde <mkl@...gutronix.de>,
	linux-can@...r.kernel.org
Cc: Thomas Kopp <thomas.kopp@...rochip.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	"Gustavo A . R . Silva" <gustavoars@...nel.org>,
	netdev@...r.kernel.org,
	linux-hardening@...r.kernel.org,
	Vincent Mailhol <mailhol.vincent@...adoo.fr>,
	Kees Cook <kees@...nel.org>
Subject: [PATCH 0/2] can: treewide: decorate flexible array members with __counted_by()

A new __counted_by() attribute was introduced in [1]. It makes the
compiler's sanitizer aware of the actual size of a flexible array
member, allowing for additional runtime checks.

I went through the full can subtree:

  * drivers/net/can
  * include/linux/can
  * include/uapi/linux/can
  * net/can

to try to identify the flexible array members that would benefit from
this attribute.

The observation is there are not so many flexible array member in the
can subtree to begin with.

Within the few flexible array members, only a two can benefit from the
__counted_by() without complex code refactoring, namely:

  * patch 1/2: struct pciefd_board from
    drivers/net/can/peak_canfd/peak_pciefd_main.c

  * patch 2/2: struct mcp251xfd_rx_ring from
    drivers/net/can/spi/mcp251xfd/mcp251xfd.h

Below is an exhaustive list of all the candidates for which
__counted_by() could not be applied to. If something did not appear
here, it just mean that I failed to catch it during my analysis.

  * All the flexible array members which rely on the
    DECLARE_FLEX_ARRAY() helper macro are out of scope for the
    moment. Those will be reconsidered depending on the output of the
    discussion in [2].

  * struct bcm_msg_head from include/uapi/linux/can/bcm.h is a special
    case. The bcm_msg_head.frames member is polymorphic: it can be
    either of:

      * an array of struct can_frame
      * an array of struct canfd_frame

    As of now, it is declared as struct can_frame for historical
    reasons. An idea is to change the type to struct canfd_frame in
    order to reflect the upper bound, in a similar fashion as struct
    mcp251xfd_rx_ring (c.f. second patch from the series). Except
    that this structure is from the uapi, meaning that such a change
    will likely break the userland, making this a bad idea.

  * struct can_skb_priv from include/linux/can/skb.h does not have a
    count member.

  * struct pucan_tx_msg and struct pucan_rx_msg from
    include/linux/can/dev/peak_canfd.h both have a flexible array
    member d, but it is counted by the four most significant bits of
    the channel_dlc member. However __counted_by() does not accept a
    shift logic. Because the layout of this structure is dictated by
    the device API, refactor is impossible here.

  * struct kvaser_usb_net_priv from
    drivers/net/can/usb/kvaser_usb/kvaser_usb.h has the
    active_tx_contexts member but it does not represent an array
    boundary. Under normal conditions, the driver may write beyond
    kvaser_usb_net_priv.tx_contexts[kvaser_usb_net_priv.active_tx_contexts].
    Code refactoring may be considered here.

  * Finally, struct flexcan_mb from
    drivers/net/can/flexcan/flexcan-core.c and struct pciefd_rx_dma
    from drivers/net/can/peak_canfd/peak_pciefd_main.c do not have
    members to indicate the count. Because the layout of these
    structures is dictated by the device API, refactor is impossible
    here.

[1] commit dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
Link: https://git.kernel.org/torvalds/c/dd06e72e68bc

[2] stddef: Allow attributes to be used when creating flex arrays
Link: https://lore.kernel.org/all/20240213234023.it.219-kees@kernel.org/T/#u

CC: Kees Cook <kees@...nel.org>

Vincent Mailhol (2):
  can: peak_canfd: decorate pciefd_board.can with __counted_by()
  can: mcp251xfd: decorate mcp251xfd_rx_ring.obj with __counted_by()

 drivers/net/can/peak_canfd/peak_pciefd_main.c | 6 ++++--
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.43.0