From: Dmitry Antipov dmantipov@yandex.ru

[ Upstream commit dcce94b80a954a8968ff29fafcfb066d6197fa9a ]

When compiling with gcc 13.1 and CONFIG_FORTIFY_SOURCE=y, I've noticed the following:

In function ‘fortify_memcpy_chk’, inlined from ‘mwifiex_construct_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:765:3, inlined from ‘mwifiex_send_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:856:6: ./include/linux/fortify-string.h:529:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning] 529 | __read_overflow2_field(q_size_field, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The compiler actually complains on:

memmove(pos + ETH_ALEN, &mgmt->u.action.category, sizeof(mgmt->u.action.u.tdls_discover_resp));

and it happens because the fortification logic interprets this as an attempt to overread 1-byte 'u.action.category' member of 'struct ieee80211_mgmt'. To silence this warning, it's enough to pass an address of 'u.action' itself instead of an address of its first member.

This also fixes an improper usage of 'sizeof()'. Since 'skb' is extended with 'sizeof(mgmt->u.action.u.tdls_discover_resp) + 1' bytes (where 1 is actually 'sizeof(mgmt->u.action.category)'), I assume that the same number of bytes should be copied.

Suggested-by: Brian Norris briannorris@chromium.org Signed-off-by: Dmitry Antipov dmantipov@yandex.ru Reviewed-by: Brian Norris briannorris@chromium.org Signed-off-by: Kalle Valo kvalo@kernel.org Link: https://lore.kernel.org/r/20230629085115.180499-2-dmantipov@yandex.ru Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/net/wireless/marvell/mwifiex/tdls.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c index f8f282ce39bd4..17f8379351923 100644 --- a/drivers/net/wireless/marvell/mwifiex/tdls.c +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c @@ -734,6 +734,7 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, int ret; u16 capab; struct ieee80211_ht_cap *ht_cap; + unsigned int extra; u8 radio, *pos;

capab = priv->curr_bss_params.bss_descriptor.cap_info_bitmap; @@ -752,7 +753,10 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv,

switch (action_code) { case WLAN_PUB_ACTION_TDLS_DISCOVER_RES: - skb_put(skb, sizeof(mgmt->u.action.u.tdls_discover_resp) + 1); + /* See the layout of 'struct ieee80211_mgmt'. */ + extra = sizeof(mgmt->u.action.u.tdls_discover_resp) + + sizeof(mgmt->u.action.category); + skb_put(skb, extra); mgmt->u.action.category = WLAN_CATEGORY_PUBLIC; mgmt->u.action.u.tdls_discover_resp.action_code = WLAN_PUB_ACTION_TDLS_DISCOVER_RES; @@ -761,8 +765,7 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, mgmt->u.action.u.tdls_discover_resp.capability = cpu_to_le16(capab); /* move back for addr4 */ - memmove(pos + ETH_ALEN, &mgmt->u.action.category, - sizeof(mgmt->u.action.u.tdls_discover_resp)); + memmove(pos + ETH_ALEN, &mgmt->u.action, extra); /* init address 4 */ eth_broadcast_addr(pos);

From: Alexander Steffen Alexander.Steffen@infineon.com

[ Upstream commit 280db21e153d8810ce3b93640c63ae922bcb9e8e ]

Similar to the transmission of TPM responses, also the transmission of TPM commands may become corrupted. Instead of aborting when detecting such issues, try resending the command again.

Signed-off-by: Alexander Steffen Alexander.Steffen@infineon.com Reviewed-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/char/tpm/tpm_tis_core.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index ef47d1d58ac3a..a084f732c1804 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -421,10 +421,17 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len) int rc; u32 ordinal; unsigned long dur; - - rc = tpm_tis_send_data(chip, buf, len); - if (rc < 0) - return rc; + unsigned int try; + + for (try = 0; try < TPM_RETRY; try++) { + rc = tpm_tis_send_data(chip, buf, len); + if (rc >= 0) + /* Data transfer done successfully */ + break; + else if (rc != -EIO) + /* Data transfer failed, not recoverable */ + return rc; + }

/* go and do it */ rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);

From: "GONG, Ruiqi" gongruiqi1@huawei.com

[ Upstream commit 3a198c95c95da10ad844cbeade2fe40bdf14c411 ]

The following message shows up when compiling with W=1:

In function ‘fortify_memcpy_chk’, inlined from ‘alx_get_ethtool_stats’ at drivers/net/ethernet/atheros/alx/ethtool.c:297:2: ./include/linux/fortify-string.h:592:4: error: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning] 592 | __read_overflow2_field(q_size_field, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In order to get alx stats altogether, alx_get_ethtool_stats() reads beyond hw->stats.rx_ok. Fix this warning by directly copying hw->stats, and refactor the unnecessarily complicated BUILD_BUG_ON btw.

Signed-off-by: GONG, Ruiqi gongruiqi1@huawei.com Reviewed-by: Simon Horman horms@kernel.org Link: https://lore.kernel.org/r/20230821013218.1614265-1-gongruiqi@huaweicloud.com Signed-off-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/net/ethernet/atheros/alx/ethtool.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c index 2f4eabf652e80..51e5aa2c74b34 100644 --- a/drivers/net/ethernet/atheros/alx/ethtool.c +++ b/drivers/net/ethernet/atheros/alx/ethtool.c @@ -281,9 +281,8 @@ static void alx_get_ethtool_stats(struct net_device *netdev, spin_lock(&alx->stats_lock);

alx_update_hw_stats(hw); - BUILD_BUG_ON(sizeof(hw->stats) - offsetof(struct alx_hw_stats, rx_ok) < - ALX_NUM_STATS * sizeof(u64)); - memcpy(data, &hw->stats.rx_ok, ALX_NUM_STATS * sizeof(u64)); + BUILD_BUG_ON(sizeof(hw->stats) != ALX_NUM_STATS * sizeof(u64)); + memcpy(data, &hw->stats, sizeof(hw->stats));

spin_unlock(&alx->stats_lock); }

From: Hao Luo haoluo@google.com

[ Upstream commit 29d67fdebc42af6466d1909c60fdd1ef4f3e5240 ]

I hit a memory leak when testing bpf_program__set_attach_target(). Basically, set_attach_target() may allocate btf_vmlinux, for example, when setting attach target for bpf_iter programs. But btf_vmlinux is freed only in bpf_object_load(), which means if we only open bpf object but not load it, setting attach target may leak btf_vmlinux.

So let's free btf_vmlinux in bpf_object__close() anyway.

Signed-off-by: Hao Luo haoluo@google.com Signed-off-by: Andrii Nakryiko andrii@kernel.org Link: https://lore.kernel.org/bpf/20230822193840.1509809-1-haoluo@google.com Signed-off-by: Sasha Levin sashal@kernel.org --- tools/lib/bpf/libbpf.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index b8849812449c3..343018632d2d1 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4202,6 +4202,7 @@ void bpf_object__close(struct bpf_object *obj) bpf_object__elf_finish(obj); bpf_object__unload(obj); btf__free(obj->btf); + btf__free(obj->btf_vmlinux); btf_ext__free(obj->btf_ext);

for (i = 0; i < obj->nr_maps; i++) {

On 9/8/2023 11:20 AM, Sasha Levin wrote:

From: Jiri Pirko jiri@nvidia.com

[ Upstream commit 633d76ad01ad0321a1ace3e5cc4fed06753d7ac4 ]

The checks in question were introduced by: commit 6b4db2e528f6 ("devlink: Fix use-after-free after a failed reload"). That fixed an issue of reload with mlxsw driver.

Back then, that was a valid fix, because there was a limitation in place that prevented drivers from registering/unregistering params when devlink instance was registered.

It was possible to do the fix differently by changing drivers to register/unregister params in appropriate places making sure the ops operate only on memory which is allocated and initialized. But that, as a dependency, would require to remove the limitation mentioned above.

Eventually, this limitation was lifted by: commit 1d18bb1a4ddd ("devlink: allow registering parameters after the instance")

Also, the alternative fix (which also fixed another issue) was done by: commit 74cbc3c03c82 ("mlxsw: spectrum_acl_tcam: Move devlink param to TCAM code").

Therefore, the checks are no longer relevant. Each driver should make sure to have the params registered only when the memory the ops are working with is allocated and initialized.

So remove the checks.

Hmmmm. Based on the description above this feels a bit odd to backport. Are we sure that its safe to remove this limitation on older kernels?

Both mentioned commits are in v6.3 so they're not in any of the stable trees by default.

Indeed grep over stable/linux-5.4.y shows nothing for either commit.

Thus, I am not convinced this is safe to backport. I didn't double check every single stable branch but given that the mentioned dependencies are in 6.3 and don't appear to have been fixes, it seems problematic for all including 5.4, 5.10, 5.15, and 6.1. No driver in those trees is going to be registering parameters early so I don't see the benefit of the patch.

Thus, it is my view this shouldn't be backported, at least not without porting the relevant dependencies as well.

Thanks, Jake

Signed-off-by: Jiri Pirko jiri@nvidia.com Reviewed-by: Ido Schimmel idosch@nvidia.com Reviewed-by: Jakub Kicinski kuba@kernel.org Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Sasha Levin sashal@kernel.org

---

net/core/devlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c index b4dabe5d89f72..5bd6330ab4275 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -2953,7 +2953,7 @@ static int devlink_param_get(struct devlink *devlink, const struct devlink_param *param, struct devlink_param_gset_ctx *ctx) {

• if (!param->get || devlink->reload_failed)

• if (!param->get) return -EOPNOTSUPP; return param->get(devlink, param->id, ctx);

} @@ -2962,7 +2962,7 @@ static int devlink_param_set(struct devlink *devlink, const struct devlink_param *param, struct devlink_param_gset_ctx *ctx) {

• if (!param->set || devlink->reload_failed)

• if (!param->set) return -EOPNOTSUPP; return param->set(devlink, param->id, ctx);

}