October 2023 - Linux-stable-mirror

[PATCH net,v2] hv_netvsc: fix race of netvsc and VF register_netdevice

by Haiyang Zhang

The rtnl lock also needs to be held before rndis_filter_device_add() which advertises nvsp_2_vsc_capability / sriov bit, and triggers VF NIC offering and registering. If VF NIC finished register_netdev() earlier it may cause name based config failure. To fix this issue, move the call to rtnl_lock() before rndis_filter_device_add(), so VF will be registered later than netvsc / synthetic NIC, and gets a name numbered (ethX) after netvsc. And, move register_netdevice_notifier() earlier, so the call back function is set before probing. Cc: stable(a)vger.kernel.org Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier in netvsc_probe()") Signed-off-by: Haiyang Zhang <haiyangz(a)microsoft.com> --- v2: Fix rtnl_unlock() in error handling as found by Wojciech Drewek. --- drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 3ba3c8fb28a5..1d1491da303b 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -2531,15 +2531,6 @@ static int netvsc_probe(struct hv_device *dev, goto devinfo_failed; } - nvdev = rndis_filter_device_add(dev, device_info); - if (IS_ERR(nvdev)) { - ret = PTR_ERR(nvdev); - netdev_err(net, "unable to add netvsc device (ret %d)\n", ret); - goto rndis_failed; - } - - eth_hw_addr_set(net, device_info->mac_adr); - /* We must get rtnl lock before scheduling nvdev->subchan_work, * otherwise netvsc_subchan_work() can get rtnl lock first and wait * all subchannels to show up, but that may not happen because @@ -2547,9 +2538,23 @@ static int netvsc_probe(struct hv_device *dev, * -> ... -> device_add() -> ... -> __device_attach() can't get * the device lock, so all the subchannels can't be processed -- * finally netvsc_subchan_work() hangs forever. + * + * The rtnl lock also needs to be held before rndis_filter_device_add() + * which advertises nvsp_2_vsc_capability / sriov bit, and triggers + * VF NIC offering and registering. If VF NIC finished register_netdev() + * earlier it may cause name based config failure. */ rtnl_lock(); + nvdev = rndis_filter_device_add(dev, device_info); + if (IS_ERR(nvdev)) { + ret = PTR_ERR(nvdev); + netdev_err(net, "unable to add netvsc device (ret %d)\n", ret); + goto rndis_failed; + } + + eth_hw_addr_set(net, device_info->mac_adr); + if (nvdev->num_chn > 1) schedule_work(&nvdev->subchan_work); @@ -2586,9 +2591,9 @@ static int netvsc_probe(struct hv_device *dev, return 0; register_failed: - rtnl_unlock(); rndis_filter_device_remove(dev, nvdev); rndis_failed: + rtnl_unlock(); netvsc_devinfo_put(device_info); devinfo_failed: free_percpu(net_device_ctx->vf_stats); @@ -2788,11 +2793,14 @@ static int __init netvsc_drv_init(void) } netvsc_ring_bytes = ring_size * PAGE_SIZE; + register_netdevice_notifier(&netvsc_netdev_notifier); + ret = vmbus_driver_register(&netvsc_drv); - if (ret) + if (ret) { + unregister_netdevice_notifier(&netvsc_netdev_notifier); return ret; + } - register_netdevice_notifier(&netvsc_netdev_notifier); return 0; } -- 2.25.1

7 months

3
3
0 0

[v5] mmc: Add quirk MMC_QUIRK_BROKEN_CACHE_FLUSH for Micron eMMC Q2J54A

by Bean Huo

From: Bean Huo <beanhuo(a)micron.com> Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache operation be allowed only after a write has occurred. Otherwise, the cache flush command or subsequent commands will time out. Signed-off-by: Bean Huo <beanhuo(a)micron.com> Signed-off-by: Rafael Beims <rafael.beims(a)toradex.com> Cc: stable(a)vger.kernel.org --- Changelog: v4--v5: 1. In the case of a successful flush, set writing_flag in _mmc_flush_cache() v3--v4: 1. Add helper function for this quirk in drivers/mmc/core/card.h. 2. Set card->written_flag only for REQ_OP_WRITE. v2--v3: 1. Set card->written_flag in mmc_blk_mq_issue_rq(). v1--v2: 1. Add Rafael's test-tag, and Co-developed-by. 2. Check host->card whether NULL or not in __mmc_start_request() before asserting host->card->->quirks --- drivers/mmc/core/block.c | 4 +++- drivers/mmc/core/card.h | 4 ++++ drivers/mmc/core/mmc.c | 8 ++++++-- drivers/mmc/core/quirks.h | 7 ++++--- include/linux/mmc/card.h | 2 ++ 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 3a8f27c3e310..152dfe593c43 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -2381,8 +2381,10 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req) } ret = mmc_blk_cqe_issue_flush(mq, req); break; - case REQ_OP_READ: case REQ_OP_WRITE: + card->written_flag = true; + fallthrough; + case REQ_OP_READ: if (host->cqe_enabled) ret = mmc_blk_cqe_issue_rw_rq(mq, req); else diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h index 4edf9057fa79..b7754a1b8d97 100644 --- a/drivers/mmc/core/card.h +++ b/drivers/mmc/core/card.h @@ -280,4 +280,8 @@ static inline int mmc_card_broken_sd_cache(const struct mmc_card *c) return c->quirks & MMC_QUIRK_BROKEN_SD_CACHE; } +static inline int mmc_card_broken_cache_flush(const struct mmc_card *c) +{ + return c->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH; +} #endif diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 8180983bd402..11053f920ac4 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -2086,13 +2086,17 @@ static int _mmc_flush_cache(struct mmc_host *host) { int err = 0; + if (mmc_card_broken_cache_flush(host->card) && !host->card->written_flag) + return err; + if (_mmc_cache_enabled(host)) { err = mmc_switch(host->card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_FLUSH_CACHE, 1, CACHE_FLUSH_TIMEOUT_MS); if (err) - pr_err("%s: cache flush error %d\n", - mmc_hostname(host), err); + pr_err("%s: cache flush error %d\n", mmc_hostname(host), err); + else + host->card->written_flag = false; } return err; diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index 32b64b564fb1..5e68c8b4cdca 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -110,11 +110,12 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = { MMC_QUIRK_TRIM_BROKEN), /* - * Micron MTFC4GACAJCN-1M advertises TRIM but it does not seems to - * support being used to offload WRITE_ZEROES. + * Micron MTFC4GACAJCN-1M supports TRIM but does not appear to suppor + * WRITE_ZEROES offloading. It also supports caching, but the cache can + * only be flushed after a write has occurred. */ MMC_FIXUP("Q2J54A", CID_MANFID_MICRON, 0x014e, add_quirk_mmc, - MMC_QUIRK_TRIM_BROKEN), + MMC_QUIRK_TRIM_BROKEN | MMC_QUIRK_BROKEN_CACHE_FLUSH), /* * Kingston EMMC04G-M627 advertises TRIM but it does not seems to diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index daa2f40d9ce6..7b12eebc5586 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -295,7 +295,9 @@ struct mmc_card { #define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */ #define MMC_QUIRK_BROKEN_SD_DISCARD (1<<14) /* Disable broken SD discard support */ #define MMC_QUIRK_BROKEN_SD_CACHE (1<<15) /* Disable broken SD cache support */ +#define MMC_QUIRK_BROKEN_CACHE_FLUSH (1<<16) /* Don't flush cache until the write has occurred */ + bool written_flag; /* Indicates eMMC has been written since power on */ bool reenable_cmdq; /* Re-enable Command Queue */ unsigned int erase_size; /* erase size in sectors */ -- 2.34.1

7 months

2
1
0 0

[PATCH] mmc: truncate quirks' oemid to 8 bits

by Dominique Martinet

We now only capture 8 bits for oemid in card->cid.oemid, so quirks that were filling up the full 16 bits up till now would no longer apply. Work around the problem by only checking for the bottom 8 bits when checking if quirks should be applied Fixes: 84ee19bffc93 ("mmc: core: Capture correct oemid-bits for eMMC cards") Link: https://lkml.kernel.org/r/ZToJsSLHr8RnuTHz@codewreck.org Signed-off-by: Dominique Martinet <dominique.martinet(a)atmark-techno.com> Cc: stable(a)vger.kernel.org Cc: Avri Altman <avri.altman(a)wdc.com> Cc: Ulf Hansson <ulf.hansson(a)linaro.org> Cc: Alex Fetters <Alex.Fetters(a)garmin.com> --- Notes: - mmc_fixup_device() was rewritten in 5.17, so older stable kernels will need a separate patch... I suppose I can send it to stable after this is merged if we go this way - struct mmc_cid's and mmc_fixup's oemid fields are unsigned shorts, we probably just want to make them unsigned char instead in which case we don't need that check anymore? But it's kind of nice to have a wider type so CID_OEMID_ANY can never be a match.... Which unfortunately my patch makes moot as ((unsigned short)-1) & 0xff will be 0xff which can match anything... - this could also be worked around in the _FIXUP_EXT macro that builds the fixup structs, but we're getting ugly here... Or we can just go for the big boom and try to fix all MMC_FIXUP() users in tree and call it a day, but that'll also be fun to backport. drivers/mmc/core/quirks.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index 32b64b564fb1..27e0349e176d 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -211,8 +211,9 @@ static inline void mmc_fixup_device(struct mmc_card *card, if (f->manfid != CID_MANFID_ANY && f->manfid != card->cid.manfid) continue; + /* Only the bottom 8bits are valid in JESD84-B51 */ if (f->oemid != CID_OEMID_ANY && - f->oemid != card->cid.oemid) + (f->oemid & 0xff) != (card->cid.oemid & 0xff)) continue; if (f->name != CID_NAME_ANY && strncmp(f->name, card->cid.prod_name, -- 2.39.2

7 months

4
5
0 0

[PATCH 1/3] cpufreq: fix broken buffer overflow detection in trans_stats

by Christian Marangi

Commit 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow") switched from snprintf to the more secure scnprintf but never updated the exit condition for PAGE_SIZE. As the commit say and as scnprintf document, what scnprintf returns what is actually written not counting the '\0' end char. This results in the case of len exceeding the size, len set to PAGE_SIZE - 1, as it can be written at max PAGESIZE - 1 (as '\0' is not counted) Because of len is never set to PAGE_SIZE, the function never break early, never print the warning and never return -EFBIG. Fix this by fixing the condition to PAGE_SIZE -1 to correctly trigger the error condition. Cc: stable(a)vger.kernel.org Fixes: 3c0897c180c6 ("cpufreq: Use scnprintf() for avoiding potential buffer overflow") Signed-off-by: Christian Marangi <ansuelsmth(a)gmail.com> --- drivers/cpufreq/cpufreq_stats.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index a33df3c66c88..40a9ff18da06 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -131,23 +131,23 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf) len += sysfs_emit_at(buf, len, " From : To\n"); len += sysfs_emit_at(buf, len, " : "); for (i = 0; i < stats->state_num; i++) { - if (len >= PAGE_SIZE) + if (len >= PAGE_SIZE - 1) break; len += sysfs_emit_at(buf, len, "%9u ", stats->freq_table[i]); } - if (len >= PAGE_SIZE) - return PAGE_SIZE; + if (len >= PAGE_SIZE - 1) + return PAGE_SIZE - 1; len += sysfs_emit_at(buf, len, "\n"); for (i = 0; i < stats->state_num; i++) { - if (len >= PAGE_SIZE) + if (len >= PAGE_SIZE - 1) break; len += sysfs_emit_at(buf, len, "%9u: ", stats->freq_table[i]); for (j = 0; j < stats->state_num; j++) { - if (len >= PAGE_SIZE) + if (len >= PAGE_SIZE - 1) break; if (pending) @@ -157,12 +157,12 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf) len += sysfs_emit_at(buf, len, "%9u ", count); } - if (len >= PAGE_SIZE) + if (len >= PAGE_SIZE - 1) break; len += sysfs_emit_at(buf, len, "\n"); } - if (len >= PAGE_SIZE) { + if (len >= PAGE_SIZE - 1) { pr_warn_once("cpufreq transition table exceeds PAGE_SIZE. Disabling\n"); return -EFBIG; } -- 2.40.1

7 months

2
5
0 0

[PATCH v1 net] net: ethtool: Fix documentation of ethtool_sprintf()

by Andrew Lunn

This function takes a pointer to a pointer, unlike sprintf() which is passed a plain pointer. Fix up the documentation to make this clear. Fixes: 7888fe53b706 ("ethtool: Add common function for filling out strings") Cc: Alexander Duyck <alexanderduyck(a)fb.com> Cc: Justin Stitt <justinstitt(a)google.com> Cc: stable(a)vger.kernel.org Signed-off-by: Andrew Lunn <andrew(a)lunn.ch> --- include/linux/ethtool.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 62b61527bcc4..1b523fd48586 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -1045,10 +1045,10 @@ static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add, /** * ethtool_sprintf - Write formatted string to ethtool string data - * @data: Pointer to start of string to update + * @data: Pointer to a pointer to the start of string to update * @fmt: Format of string to write * - * Write formatted string to data. Update data to point at start of + * Write formatted string to *data. Update *data to point at start of * next string. */ extern __printf(2, 3) void ethtool_sprintf(u8 **data, const char *fmt, ...); -- 2.42.0

7 months

3
2
0 0

[PATCH] coresight: etm4x: Fix width of CCITMIN field

by James Clark

CCITMIN is a 12 bit field and doesn't fit in a u8, so extend it to u16. This probably wasn't an issue previously because values higher than 255 never occurred. But since commit 0f55b43dedcd ("coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus"), a comparison with 256 was done to enable the errata, generating the following W=1 build error: coresight-etm4x-core.c:1188:24: error: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare] if (drvdata->ccitmin == 256) Cc: stable(a)vger.kernel.org Fixes: 54ff892b76c6 ("coresight: etm4x: splitting struct etmv4_drvdata") Signed-off-by: James Clark <james.clark(a)arm.com> --- drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 20e2e4cb7614..da17b6c49b0f 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -1036,7 +1036,7 @@ struct etmv4_drvdata { u8 ctxid_size; u8 vmid_size; u8 ccsize; - u8 ccitmin; + u16 ccitmin; u8 s_ex_level; u8 ns_ex_level; u8 q_support; -- 2.34.1

7 months

3
3
0 0

[PATCH v3] serial: 8250: 8250_omap: Clear UART_HAS_RHR_IT_DIS bit

by Ronald Wahl

From: Ronald Wahl <ronald.wahl(a)raritan.com> This fixes commit 439c7183e5b9 ("serial: 8250: 8250_omap: Disable RX interrupt after DMA enable") which unfortunately set the UART_HAS_RHR_IT_DIS bit in the UART_OMAP_IER2 register and never cleared it. Cc: stable(a)vger.kernel.org Fixes: 439c7183e5b9 ("serial: 8250: 8250_omap: Disable RX interrupt after DMA enable") Signed-off-by: Ronald Wahl <ronald.wahl(a)raritan.com> --- V3: - add Cc: stable(a)vger.kernel.org V2: - add Fixes: tag - fix author drivers/tty/serial/8250/8250_omap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index ca972fd37725..c7ab2963040b 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -914,7 +914,7 @@ static void __dma_rx_do_complete(struct uart_8250_port *p) if (priv->habit & UART_HAS_RHR_IT_DIS) { reg = serial_in(p, UART_OMAP_IER2); reg &= ~UART_OMAP_IER2_RHR_IT_DIS; - serial_out(p, UART_OMAP_IER2, UART_OMAP_IER2_RHR_IT_DIS); + serial_out(p, UART_OMAP_IER2, reg); } dmaengine_tx_status(rxchan, cookie, &state); @@ -1060,7 +1060,7 @@ static int omap_8250_rx_dma(struct uart_8250_port *p) if (priv->habit & UART_HAS_RHR_IT_DIS) { reg = serial_in(p, UART_OMAP_IER2); reg |= UART_OMAP_IER2_RHR_IT_DIS; - serial_out(p, UART_OMAP_IER2, UART_OMAP_IER2_RHR_IT_DIS); + serial_out(p, UART_OMAP_IER2, reg); } dma_async_issue_pending(dma->rxchan); -- 2.41.0

7 months

2
1
0 0

Possibly broken Linux 5.10.198 backport spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe

by Marek Vasut

Linux 5.10.198 commit 2cdec9c13f81 ("spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe") looks very different compared to matching upstream commit: a21fbc42807b ("spi: spi-zynqmp-gqspi: Fix runtime PM imbalance in zynqmp_qspi_probe") The Linux 5.10.198 change breaks a platform for me and it really looks like an incorrect backport. Dinghao, can you have a look ? Thank you

7 months

4
4
0 0

[REGRESSION] Userland interface breaks due to hard HFSC_FSC requirement

by Christian Theune

Hi, (prefix, I was not aware of the regression reporting process and incorrectly reported this informally with the developers mentioned in the change) I upgraded from 6.1.38 to 6.1.55 this morning and it broke my traffic shaping script, leaving me with a non-functional uplink on a remote router. The script errors out like this: Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + ext=ispA Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + ext_ingress=ifb0 Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + modprobe ifb Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + modprobe act_mirred Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc del dev ispA root Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2061]: Error: Cannot delete qdisc with handle of zero. Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + true Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc del dev ispA ingress Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2064]: Error: Cannot find specified qdisc on specified device. Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + true Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc del dev ifb0 root Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2066]: Error: Cannot delete qdisc with handle of zero. Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + true Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc del dev ifb0 ingress Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2067]: Error: Cannot find specified qdisc on specified device. Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + true Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc add dev ispA handle ffff: ingress Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + ifconfig ifb0 up Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc filter add dev ispA parent ffff: protocol all u32 match u32 0 0 action mirred egress redirect dev ifb0 Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc qdisc add dev ifb0 root handle 1: hfsc default 1 Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc class add dev ifb0 parent 1: classid 1:999 hfsc rt m2 2.5gbit Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2053]: + tc class add dev ifb0 parent 1:999 classid 1:1 hfsc sc rate 50mbit Oct 06 05:49:22 wendy00 isp-setup-shaping-start[2077]: Error: Invalid parent - parent class must have FSC. The error message is also a bit weird (but that’s likely due to iproute2 being weird) as the CLI interface for `tc` and the error message do not map well. (I think I would have to choose `hfsc sc` on the parent to enable the FSC option which isn’t mentioned anywhere in the hfsc manpage). The breaking change was introduced in 6.1.53[1] and a multitude of other currently supported kernels: ---- commit a1e820fc7808e42b990d224f40e9b4895503ac40 Author: Budimir Markovic <markovicbudimir(a)gmail.com> Date: Thu Aug 24 01:49:05 2023 -0700 net/sched: sch_hfsc: Ensure inner classes have fsc curve [ Upstream commit b3d26c5702c7d6c45456326e56d2ccf3f103e60f ] HFSC assumes that inner classes have an fsc curve, but it is currently possible for classes without an fsc curve to become parents. This leads to bugs including a use-after-free. Don't allow non-root classes without HFSC_FSC to become parents. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Budimir Markovic <markovicbudimir(a)gmail.com> Signed-off-by: Budimir Markovic <markovicbudimir(a)gmail.com> Acked-by: Jamal Hadi Salim <jhs(a)mojatatu.com> Link: https://lore.kernel.org/r/20230824084905.422-1-markovicbudimir@gmail.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> ---- Regards, Christian [1] https://cdn.kernel.org/pub/linux/kernel/v6.x/ChangeLog-6.1.53 #regzbot introduced: a1e820fc7808e42b990d224f40e9b4895503ac40 -- Christian Theune · ct(a)flyingcircus.io · +49 345 219401 0 Flying Circus Internet Operations GmbH · https://flyingcircus.io Leipziger Str. 70/71 · 06108 Halle (Saale) · Deutschland HR Stendal HRB 21169 · Geschäftsführer: Christian Theune, Christian Zagrodnick

7 months

7
23
0 0

[PATCH net v2] llc: verify mac len before reading mac header

by Willem de Bruijn

From: Willem de Bruijn <willemb(a)google.com> LLC reads the mac header with eth_hdr without verifying that the skb has an Ethernet header. Syzbot was able to enter llc_rcv on a tun device. Tun can insert packets without mac len and with user configurable skb->protocol (passing a tun_pi header when not configuring IFF_NO_PI). BUG: KMSAN: uninit-value in llc_station_ac_send_test_r net/llc/llc_station.c:81 [inline] BUG: KMSAN: uninit-value in llc_station_rcv+0x6fb/0x1290 net/llc/llc_station.c:111 llc_station_ac_send_test_r net/llc/llc_station.c:81 [inline] llc_station_rcv+0x6fb/0x1290 net/llc/llc_station.c:111 llc_rcv+0xc5d/0x14a0 net/llc/llc_input.c:218 __netif_receive_skb_one_core net/core/dev.c:5523 [inline] __netif_receive_skb+0x1a6/0x5a0 net/core/dev.c:5637 netif_receive_skb_internal net/core/dev.c:5723 [inline] netif_receive_skb+0x58/0x660 net/core/dev.c:5782 tun_rx_batched+0x3ee/0x980 drivers/net/tun.c:1555 tun_get_user+0x54c5/0x69c0 drivers/net/tun.c:2002 Add a mac_len test before all three eth_hdr(skb) calls under net/llc. There are further uses in include/net/llc_pdu.h. All these are protected by a test skb->protocol == ETH_P_802_2. Which does not protect against this tun scenario. But the mac_len test added in this patch in llc_fixup_skb will indirectly protect those too. That is called from llc_rcv before any other LLC code. It is tempting to just add a blanket mac_len check in llc_rcv, but not sure whether that could break valid LLC paths that do not assume an Ethernet header. 802.2 LLC may be used on top of non-802.3 protocols in principle. The below referenced commit shows that used to, on top of Token Ring. At least one of the three eth_hdr uses goes back to before the start of git history. But the one that syzbot exercises is introduced in this commit. That commit is old enough (2008), that effectively all stable kernels should receive this. Fixes: f83f1768f833 ("[LLC]: skb allocation size for responses") Reported-by: syzbot+a8c7be6dee0de1b669cc(a)syzkaller.appspotmail.com Signed-off-by: Willem de Bruijn <willemb(a)google.com> --- Changes v1->v2 - fix return value in llc_sap_action_send_test_r - add Fixes tag - cc: stable(a)vger.kernel.org --- net/llc/llc_input.c | 10 ++++++++-- net/llc/llc_s_ac.c | 3 +++ net/llc/llc_station.c | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/net/llc/llc_input.c b/net/llc/llc_input.c index 7cac441862e21..51bccfb00a9cd 100644 --- a/net/llc/llc_input.c +++ b/net/llc/llc_input.c @@ -127,8 +127,14 @@ static inline int llc_fixup_skb(struct sk_buff *skb) skb->transport_header += llc_len; skb_pull(skb, llc_len); if (skb->protocol == htons(ETH_P_802_2)) { - __be16 pdulen = eth_hdr(skb)->h_proto; - s32 data_size = ntohs(pdulen) - llc_len; + __be16 pdulen; + s32 data_size; + + if (skb->mac_len < ETH_HLEN) + return 0; + + pdulen = eth_hdr(skb)->h_proto; + data_size = ntohs(pdulen) - llc_len; if (data_size < 0 || !pskb_may_pull(skb, data_size)) diff --git a/net/llc/llc_s_ac.c b/net/llc/llc_s_ac.c index 79d1cef8f15a9..06fb8e6944b06 100644 --- a/net/llc/llc_s_ac.c +++ b/net/llc/llc_s_ac.c @@ -153,6 +153,9 @@ int llc_sap_action_send_test_r(struct llc_sap *sap, struct sk_buff *skb) int rc = 1; u32 data_size; + if (skb->mac_len < ETH_HLEN) + return 1; + llc_pdu_decode_sa(skb, mac_da); llc_pdu_decode_da(skb, mac_sa); llc_pdu_decode_ssap(skb, &dsap); diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c index 05c6ae0920534..f506542925109 100644 --- a/net/llc/llc_station.c +++ b/net/llc/llc_station.c @@ -76,6 +76,9 @@ static int llc_station_ac_send_test_r(struct sk_buff *skb) u32 data_size; struct sk_buff *nskb; + if (skb->mac_len < ETH_HLEN) + goto out; + /* The test request command is type U (llc_len = 3) */ data_size = ntohs(eth_hdr(skb)->h_proto) - 3; nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, data_size); -- 2.42.0.758.gaed0368e0e-goog

7 months

2
1
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror October 2023