5.14.4-rc1 review
Subsystems (2)
Greg Kroah-Hartman
maintainerPATCH 278/334 net: phy: marvell10g: fix broken PHY interrupts for anyone after us in the driver probe list
In reply to Greg Kroah-HartmanFrom: Vladimir Oltean <[email protected]>
[ Upstream commit 0d55649d2ad7296acfda9127e1d05518d025734a ]
Enabling interrupts via device tree for the internal PHYs on the mv88e6390 DSA switch does not work. The driver insists to use poll mode.
Stage one debugging shows that the fwnode_mdiobus_phy_device_register function calls fwnode_irq_get properly, and phy->irq is set to a valid interrupt line initially.
But it is then cleared.
Stage two debugging shows that it is cleared here:
phy_probe:
/* Disable the interrupt if the PHY doesn't support it
* but the interrupt is still a valid one
*/
if (!phy_drv_supports_irq(phydrv) && phy_interrupt_is_valid(phydev))
phydev->irq = PHY_POLL;
Okay, so does the "Marvell 88E6390 Family" PHY driver not have the .config_intr and .handle_interrupt function pointers? Yes it does.
Stage three debugging shows that the PHY device does not attempt a probe against the "Marvell 88E6390 Family" driver, but against the "mv88x3310" driver.
Okay, so why does the "mv88x3310" driver match on a mv88x6390 internal PHY? The PHY IDs (MARVELL_PHY_ID_88E6390_FAMILY vs MARVELL_PHY_ID_88X3310) are way different.
Stage four debugging has us looking through:
phy_device_register
-> device_add
-> bus_probe_device
-> device_initial_probe
-> __device_attach
-> bus_for_each_drv
-> driver_match_device
-> drv->bus->match
-> phy_bus_matchOkay, so as we said, the MII_PHYSID1 of mv88e6390 does not match the mv88x3310 driver's PHY mask & ID, so why would phy_bus_match return...
Ahh, phy_bus_match calls a shortcircuit method, phydrv->match_phy_device, and does not even bother to compare the PHY ID if that is implemented.
So of course, we go inside the marvell10g.c driver and sure enough, it implements .match_phy_device and does not bother to check the PHY ID.
What's interesting though is that at the end of the device_add() from phy_device_register(), the driver for the internal PHYs _is_ the proper "Marvell 88E6390 Family". This is because "mv88x3310" ends up failing to probe after all, and __device_attach_driver(), to quote:
/*
* Ignore errors returned by ->probe so that the next driver can try
* its luck.
*/
The next (and only other) driver that matches is the 6390 driver. For this one, phy_probe doesn't fail, and everything expects to work as normal, EXCEPT phydev->irq has already been cleared by the previous unsuccessful probe of a driver which did not implement PHY interrupts, and therefore cleared that IRQ.
Okay, so it is not just Marvell 6390 that has PHY interrupts broken. Stuff like Atheros, Aquantia, Broadcom, Qualcomm work because they are lexicographically before Marvell, and stuff like NXP, Realtek, Vitesse are broken.
This goes to show how fragile it is to reset phydev->irq = PHY_POLL from the actual beginning of phy_probe itself. That seems like an actual bug of its own too, since phy_probe has side effects which are not restored on probe failure, but the line of thought probably was, the same driver will attempt probe again, so it doesn't matter. Well, looks like it does.
Maybe it would make more sense to move the phydev->irq clearing after the actual device_add() in phy_device_register() completes, and the bound driver is the actual final one.
(also, a bit frightening that drivers are permitted to bypass the MDIO bus matching in such a trivial way and perform PHY reads and writes from the .match_phy_device method, on devices that do not even belong to them. In the general case it might not be guaranteed that the MDIO accesses one driver needs to make to figure out whether to match on a device is safe for all other PHY devices)
--- drivers/net/phy/marvell10g.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/net/phy/marvell10g.c +8 -0
@@ -987,11 +987,19 @@ static int mv3310_get_number_of_ports(struct phy_device *phydev)static int mv3310_match_phy_device (struct phy_device *phydev) { +if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD ] & +MARVELL_PHY_ID_MASK ) !=MARVELL_PHY_ID_88X3310 ) +return 0 ; +return mv3310_get_number_of_ports (phydev) ==1 ; }static int mv3340_match_phy_device (struct phy_device *phydev) { +if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD ] & +MARVELL_PHY_ID_MASK ) !=MARVELL_PHY_ID_88X3310 ) +return 0 ; +return mv3310_get_number_of_ports (phydev) ==4 ; }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 279/334 ath6kl: wmi: fix an error code in ath6kl_wmi_sync_point()
In reply to Greg Kroah-HartmanFrom: Dan Carpenter <[email protected]>
[ Upstream commit fd6729ec534cffbbeb3917761e6d1fe6a412d3fe ]
This error path is unlikely because of it checked for NULL and returned -ENOMEM earlier in the function. But it should return an error code here as well if we ever do hit it because of a race condition or something.
--- drivers/net/wireless/ath/ath6kl/wmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/wireless/ath/ath6kl/wmi.c +3 -1
@@ -2504,8 +2504,10 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) goto free_data_skb;for (index =0 ; index < num_pri_streams; index++) { -if (WARN_ON (!data_sync_bufs[index].skb)) +if (WARN_ON (!data_sync_bufs[index].skb)) { + ret = -ENOMEM ; goto free_data_skb; + } ep_id = ath6kl_ac2_endpoint_id(wmi->parent_dev, data_sync_bufs[index].
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 280/334 ALSA: usb-audio: Add lowlatency module option
In reply to Greg Kroah-HartmanFrom: Takashi Iwai <[email protected]>
[ Upstream commit 4801bee7d5a36c199b734a28cde5259183aff822 ]
For making user to switch back to the old playback mode, this patch adds a new module option 'lowlatency' to snd-usb-audio driver. When user face a regression due to the recent low-latency playback support, they can test easily by passing lowlatency=0 option without rebuilding the kernel.
--- sound/usb/card.c | 4 ++++ sound/usb/pcm.c | 3 ++- sound/usb/usbaudio.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)
sound/usb/card.c +4 -0
@@ -68,6 +68,7 @@ static int pid[SNDRV_CARDS] = { [0 ... (SNDRV_CARDS-1)] = -1 };static int device_setup[SNDRV_CARDS ];/* device parameter for this card */ static bool ignore_ctl_error;static bool autoclock = true; +static bool lowlatency = true;static char *quirk_alias[SNDRV_CARDS ];static char *delayed_register[SNDRV_CARDS ];static bool implicit_fb[SNDRV_CARDS ];
@@ -92,6 +93,8 @@ MODULE_PARM_DESC(ignore_ctl_error, "Ignore errors from USB controller for mixer interfaces.");module_param (autoclock, bool,0444 );MODULE_PARM_DESC (autoclock,"Enable auto-clock selection for UAC2 devices (default: yes)." ); +module_param (lowlatency, bool,0444 ); +MODULE_PARM_DESC (lowlatency,"Enable low latency playback (default: yes)." );module_param_array (quirk_alias, charp,NULL ,0444 );MODULE_PARM_DESC (quirk_alias,"Quirk aliases, e.g. 0123abcd:5678beef." );module_param_array (delayed_register, charp,NULL ,0444 );
@@ -599,6 +602,7 @@ static int snd_usb_audio_create(struct usb_interface *intf, chip->setup = device_setup[idx]; chip->generic_implicit_fb = implicit_fb[idx]; chip->autoclock = autoclock; + chip->lowlatency = lowlatency;atomic_set (&chip->active,1 );/* avoid autopm during probing */ atomic_set (&chip->usage_count,0 );atomic_set (&chip->shutdown,0 );
sound/usb/pcm.c +2 -1
@@ -617,7 +617,8 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)/* check whether early start is needed for playback stream */ subs->early_playback_start = subs->direction ==SNDRV_PCM_STREAM_PLAYBACK && - subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes; + (!chip->lowlatency || + (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes));if (subs->early_playback_start) ret =start_endpoints (subs);
sound/usb/usbaudio.h +1 -0
@@ -57,6 +57,7 @@ struct snd_usb_audio {bool generic_implicit_fb;/* from the 'implicit_fb' module param */ bool autoclock;/* from the 'autoclock' module param */ +bool lowlatency;/* from the 'lowlatency' module param */ struct usb_host_interface *ctrl_intf;/* the audio control interface */ struct media_device *media_dev;struct media_intf_devnode *ctl_intf_media_devnode;
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 281/334 atlantic: Fix driver resume flow.
In reply to Greg Kroah-HartmanFrom: Sudarsana Reddy Kalluru <[email protected]>
[ Upstream commit 57f780f1c43362b86fd23d20bd940e2468237716 ]
Driver crashes when restoring from the Hibernate. In the resume flow, driver need to clean up the older nic/vec objects and re-initialize them.
--- drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c +3 -0
@@ -417,6 +417,9 @@ static int atl_resume_common(struct device *dev, bool deep)pci_restore_state (pdev);if (deep) { +/* Reinitialize Nic/Vecs objects */ +aq_nic_deinit (nic, !nic->aq_hw->aq_nic_cfg->wol); + ret =aq_nic_init (nic);if (ret) goto err_exit;
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 282/334 bcma: Fix memory leak for internally-handled cores
In reply to Greg Kroah-HartmanFrom: Zenghui Yu <[email protected]>
[ Upstream commit b63aed3ff195130fef12e0af590f4838cf0201d8 ]
kmemleak reported that dev_name() of internally-handled cores were leaked on driver unbinding. Let's use device_initialize() to take refcounts for them and put_device() to properly free the related stuff.
While looking at it, there's another potential issue for those which should be *registered* into driver core. If device_register() failed, we put device once and freed bcma_device structures. In bcma_unregister_cores(), they're treated as unregistered and we hit both UAF and double-free. That smells not good and has also been fixed now.
--- drivers/bcma/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/bcma/main.c +3 -3
@@ -236,6 +236,7 @@ EXPORT_SYMBOL(bcma_core_irq);void bcma_prepare_core (struct bcma_bus *bus,struct bcma_device *core) { +device_initialize (&core->dev); core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type;dev_set_name (&core->dev,"bcma%d:%d" , bus->num, core->core_index);
@@ -277,11 +278,10 @@ static void bcma_register_core(struct bcma_bus *bus, struct bcma_device *core) {int err; - err =device_register (&core->dev); + err =device_add (&core->dev);if (err) {bcma_err (bus,"Could not register dev for core 0x%03X\n" , core->id.id); -put_device (&core->dev);return ; } core->dev_registered = true;
@@ -372,7 +372,7 @@ void bcma_unregister_cores(struct bcma_bus *bus)/* Now noone uses internally-handled cores, we can free them */ list_for_each_entry_safe (core, tmp, &bus->cores, list) {list_del (&core->list); -kfree (core); +put_device (&core->dev); } }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 283/334 brcmfmac: pcie: fix oops on failure to resume and reprobe
In reply to Greg Kroah-HartmanFrom: Ahmad Fatoum <[email protected]>
[ Upstream commit d745ca4f2c4ae9f1bd8cf7d8ac6e22d739bffd19 ]
When resuming from suspend, brcmf_pcie_pm_leave_D3 will first attempt a hot resume and then fall back to removing the PCI device and then reprobing. If this probe fails, the kernel will oops, because brcmf_err, which is called to report the failure will dereference the stale bus pointer. Open code and use the default bus-less brcmf_err to avoid this.
--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +1 -1
@@ -2075,7 +2075,7 @@ cleanup: err =brcmf_pcie_probe (pdev,NULL );if (err) -brcmf_err (bus,"probe after resume failed, err=%d\n" , err); +__brcmf_err (NULL , __func__,"probe after resume failed, err=%d\n" , err);return err; }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 284/334 ipv6: make exception cache less predictible
In reply to Greg Kroah-HartmanFrom: Eric Dumazet <[email protected]>
[ Upstream commit a00df2caffed3883c341d5685f830434312e4a43 ]
Even after commit 4785305c05b2 ("ipv6: use siphash in rt6_exception_hash()"), an attacker can still use brute force to learn some secrets from a victim linux host.
One way to defeat these attacks is to make the max depth of the hash table bucket a random value.
Before this patch, each bucket of the hash table used to store exceptions could contain 6 items under attack.
After the patch, each bucket would contains a random number of items, between 6 and 10. The attacker can no longer infer secrets.
This is slightly increasing memory size used by the hash table, we do not expect this to be a problem.
Following patch is dealing with the same issue in IPv4.
--- net/ipv6/route.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
net/ipv6/route.c +4 -1
@@ -1657,6 +1657,7 @@ static int rt6_insert_exception(struct rt6_info *nrt,struct in6_addr *src_key =NULL ;struct rt6_exception *rt6_ex;struct fib6_nh *nh = res->nh; +int max_depth;int err =0 ;spin_lock_bh (&rt6_exception_lock);
@@ -1711,7 +1712,9 @@ static int rt6_insert_exception(struct rt6_info *nrt, bucket->depth++; net->ipv6.rt6_stats->fib_rt_cache++; -if (bucket->depth >FIB6_MAX_DEPTH ) +/* Randomize max depth to avoid some side channels attacks. */ + max_depth =FIB6_MAX_DEPTH +prandom_u32_max (FIB6_MAX_DEPTH ); +while (bucket->depth > max_depth)rt6_exception_remove_oldest (bucket); out:
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 285/334 ipv4: make exception cache less predictible
In reply to Greg Kroah-HartmanFrom: Eric Dumazet <[email protected]>
[ Upstream commit 67d6d681e15b578c1725bad8ad079e05d1c48a8e ]
Even after commit 6457378fe796 ("ipv4: use siphash instead of Jenkins in fnhe_hashfun()"), an attacker can still use brute force to learn some secrets from a victim linux host.
One way to defeat these attacks is to make the max depth of the hash table bucket a random value.
Before this patch, each bucket of the hash table used to store exceptions could contain 6 items under attack.
After the patch, each bucket would contains a random number of items, between 6 and 10. The attacker can no longer infer secrets.
This is slightly increasing memory size used by the hash table, by 50% in average, we do not expect this to be a problem.
This patch is more complex than the prior one (IPv6 equivalent), because IPv4 was reusing the oldest entry.
Since we need to be able to evict more than one entry per update_or_create_fnhe() call, I had to replace fnhe_oldest() with fnhe_remove_oldest().
Also note that we will queue extra kfree_rcu() calls under stress, which hopefully wont be a too big issue.
--- net/ipv4/route.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-)
net/ipv4/route.c +30 -16
@@ -586,18 +586,25 @@ static void fnhe_flush_routes(struct fib_nh_exception *fnhe) } } -static struct fib_nh_exception *fnhe_oldest (struct fnhe_hash_bucket *hash) +static void fnhe_remove_oldest (struct fnhe_hash_bucket *hash) { -struct fib_nh_exception *fnhe, *oldest; +struct fib_nh_exception __rcu **fnhe_p, **oldest_p; +struct fib_nh_exception *fnhe, *oldest =NULL ; - oldest =rcu_dereference (hash->chain); -for (fnhe =rcu_dereference (oldest->fnhe_next); fnhe; - fnhe =rcu_dereference (fnhe->fnhe_next)) { -if (time_before (fnhe->fnhe_stamp, oldest->fnhe_stamp)) +for (fnhe_p = &hash->chain; ; fnhe_p = &fnhe->fnhe_next) { + fnhe =rcu_dereference_protected (*fnhe_p, +lockdep_is_held (&fnhe_lock)); +if (!fnhe) +break ; +if (!oldest || +time_before (fnhe->fnhe_stamp, oldest->fnhe_stamp)) { oldest = fnhe; + oldest_p = fnhe_p; + } }fnhe_flush_routes (oldest); -return oldest; + *oldest_p = oldest->fnhe_next; +kfree_rcu (oldest, rcu); }static u32 fnhe_hashfun (__be32 daddr)
@@ -676,16 +683,21 @@ static void update_or_create_fnhe(struct fib_nh_common *nhc, __be32 daddr,if (rt)fill_route_from_fnhe (rt, fnhe); }else { -if (depth >FNHE_RECLAIM_DEPTH ) - fnhe =fnhe_oldest (hash); -else { - fnhe =kzalloc (sizeof (*fnhe),GFP_ATOMIC ); -if (!fnhe) - goto out_unlock; - - fnhe->fnhe_next = hash->chain; -rcu_assign_pointer (hash->chain, fnhe); +/* Randomize max depth to avoid some side channels attacks. */ +int max_depth =FNHE_RECLAIM_DEPTH + +prandom_u32_max (FNHE_RECLAIM_DEPTH ); + +while (depth > max_depth) { +fnhe_remove_oldest (hash); + depth--; } + + fnhe =kzalloc (sizeof (*fnhe),GFP_ATOMIC ); +if (!fnhe) + goto out_unlock; + + fnhe->fnhe_next = hash->chain; + fnhe->fnhe_genid = genid; fnhe->fnhe_daddr = daddr; fnhe->fnhe_gw = gw;
@@ -693,6 +705,8 @@ static void update_or_create_fnhe(struct fib_nh_common *nhc, __be32 daddr, fnhe->fnhe_mtu_locked = lock; fnhe->fnhe_expires =max (1UL , expires); +rcu_assign_pointer (hash->chain, fnhe); +/* Exception created; mark the cached routes for the nexthop * stale, so anyone caching it rechecks if this exception * applies to them.
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 286/334 net: qrtr: make checks in qrtr_endpoint_post() stricter
In reply to Greg Kroah-HartmanFrom: Dan Carpenter <[email protected]>
[ Upstream commit aaa8e4922c887ff47ad66ef918193682bccc1905 ]
These checks are still not strict enough. The main problem is that if "cb->type == QRTR_TYPE_NEW_SERVER" is true then "len - hdrlen" is guaranteed to be 4 but we need to be at least 16 bytes. In fact, we can reject everything smaller than sizeof(*pkt) which is 20 bytes.
Also I don't like the ALIGN(size, 4). It's better to just insist that data is needs to be aligned at the start.
--- net/qrtr/qrtr.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
net/qrtr/qrtr.c +6 -2
@@ -493,7 +493,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) goto err; } -if (!size || len !=ALIGN (size,4 ) + hdrlen) +if (!size || size &3 || len != size + hdrlen) goto err; if (cb->dst_port != QRTR_PORT_CTRL && cb->type != QRTR_TYPE_DATA &&
@@ -506,8 +506,12 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)if (cb->type ==QRTR_TYPE_NEW_SERVER ) {/* Remote node endpoint can bridge other distant nodes */ -const struct qrtr_ctrl_pkt *pkt = data + hdrlen; +const struct qrtr_ctrl_pkt *pkt; +if (size <sizeof (*pkt)) + goto err; + + pkt = data + hdrlen;qrtr_node_assign (node,le32_to_cpu (pkt->server.node)); }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 287/334 sch_htb: Fix inconsistency when leaf qdisc creation fails
In reply to Greg Kroah-HartmanFrom: Maxim Mikityanskiy <[email protected]>
[ Upstream commit ca49bfd90a9dde175d2929dc1544b54841e33804 ]
In HTB offload mode, qdiscs of leaf classes are grafted to netdev queues. sch_htb expects the dev_queue field of these qdiscs to point to the corresponding queues. However, qdisc creation may fail, and in that case noop_qdisc is used instead. Its dev_queue doesn't point to the right queue, so sch_htb can lose track of used netdev queues, which will cause internal inconsistencies.
This commit fixes this bug by keeping track of the netdev queue inside struct htb_class. All reads of cl->leaf.q->dev_queue are replaced by the new field, the two values are synced on writes, and WARNs are added to assert equality of the two values.
The driver API has changed: when TC_HTB_LEAF_DEL needs to move a queue, the driver used to pass the old and new queue IDs to sch_htb. Now that there is a new field (offload_queue) in struct htb_class that needs to be updated on this operation, the driver will pass the old class ID to sch_htb instead (it already knows the new class ID).
--- .../net/ethernet/mellanox/mlx5/core/en/qos.c | 15 ++- .../net/ethernet/mellanox/mlx5/core/en/qos.h | 4 +- .../net/ethernet/mellanox/mlx5/core/en_main.c | 3 +- include/net/pkt_cls.h | 3 +- net/sched/sch_htb.c | 97 ++++++++++++------- 5 files changed, 72 insertions(+), 50 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en/qos.c +6 -9
@@ -733,8 +733,8 @@ static void mlx5e_reset_qdisc(struct net_device *dev, u16 qid)spin_unlock_bh (qdisc_lock (qdisc)); } -int mlx5e_htb_leaf_del (struct mlx5e_priv *priv,u16 classid,u16 *old_qid, -u16 *new_qid,struct netlink_ext_ack *extack) +int mlx5e_htb_leaf_del (struct mlx5e_priv *priv,u16 *classid, +struct netlink_ext_ack *extack) {struct mlx5e_qos_node *node;struct netdev_queue *txq;
@@ -742,11 +742,9 @@ int mlx5e_htb_leaf_del(struct mlx5e_priv *priv, u16 classid, u16 *old_qid,bool opened;int err; -qos_dbg (priv->mdev,"TC_HTB_LEAF_DEL classid %04x\n" , classid); - - *old_qid = *new_qid =0 ; +qos_dbg (priv->mdev,"TC_HTB_LEAF_DEL classid %04x\n" , *classid); - node =mlx5e_sw_node_find (priv, classid); + node =mlx5e_sw_node_find (priv, *classid);if (!node)return -ENOENT ;
@@ -764,7 +762,7 @@ int mlx5e_htb_leaf_del(struct mlx5e_priv *priv, u16 classid, u16 *old_qid, err =mlx5_qos_destroy_node (priv->mdev, node->hw_id);if (err)/* Not fatal. */ qos_warn (priv->mdev,"Failed to destroy leaf node %u (class %04x), err = %d\n" , - node->hw_id, classid, err); + node->hw_id, *classid, err);mlx5e_sw_node_delete (priv, node);
@@ -826,8 +824,7 @@ int mlx5e_htb_leaf_del(struct mlx5e_priv *priv, u16 classid, u16 *old_qid,if (opened)mlx5e_reactivate_qos_sq (priv, moved_qid, txq); - *old_qid =mlx5e_qid_from_qos (&priv->channels, moved_qid); - *new_qid =mlx5e_qid_from_qos (&priv->channels, qid); + *classid = node->classid;return 0 ; }
drivers/net/ethernet/mellanox/mlx5/core/en/qos.h +2 -2
@@ -34,8 +34,8 @@ int mlx5e_htb_leaf_alloc_queue(struct mlx5e_priv *priv, u16 classid, struct netlink_ext_ack *extack);int mlx5e_htb_leaf_to_inner (struct mlx5e_priv *priv,u16 classid,u16 child_classid,u64 rate,u64 ceil,struct netlink_ext_ack *extack); -int mlx5e_htb_leaf_del (struct mlx5e_priv *priv,u16 classid,u16 *old_qid, -u16 *new_qid,struct netlink_ext_ack *extack); +int mlx5e_htb_leaf_del (struct mlx5e_priv *priv,u16 *classid, +struct netlink_ext_ack *extack);int mlx5e_htb_leaf_del_last (struct mlx5e_priv *priv,u16 classid,bool force,struct netlink_ext_ack *extack); int mlx5e_htb_node_modify(struct mlx5e_priv *priv, u16 classid, u64 rate, u64 ceil,
drivers/net/ethernet/mellanox/mlx5/core/en_main.c +1 -2
@@ -3453,8 +3453,7 @@ static int mlx5e_setup_tc_htb(struct mlx5e_priv *priv, struct tc_htb_qopt_offloareturn mlx5e_htb_leaf_to_inner (priv, htb->parent_classid, htb->classid, htb->rate, htb->ceil, htb->extack);case TC_HTB_LEAF_DEL : -return mlx5e_htb_leaf_del (priv, htb->classid, &htb->moved_qid, &htb->qid, - htb->extack); +return mlx5e_htb_leaf_del (priv, &htb->classid, htb->extack);case TC_HTB_LEAF_DEL_LAST :case TC_HTB_LEAF_DEL_LAST_FORCE : return mlx5e_htb_leaf_del_last(priv, htb->classid,
include/net/pkt_cls.h +1 -2
@@ -824,10 +824,9 @@ enum tc_htb_command {struct tc_htb_qopt_offload {struct netlink_ext_ack *extack;enum tc_htb_command command; -u16 classid;u32 parent_classid; +u16 classid;u16 qid; -u16 moved_qid;u64 rate;u64 ceil; };
net/sched/sch_htb.c +62 -35
170 lines not on this page · raw message
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 288/334 net: sched: Fix qdisc_rate_table refcount leak when get tcf_block failed
In reply to Greg Kroah-HartmanFrom: Xiyu Yang <[email protected]>
[ Upstream commit c66070125837900163b81a03063ddd657a7e9bfb ]
The reference counting issue happens in one exception handling path of cbq_change_class(). When failing to get tcf_block, the function forgets to decrease the refcount of "rtab" increased by qdisc_put_rtab(), causing a refcount leak.
Fix this issue by jumping to "failure" label when get tcf_block failed.
--- net/sched/sch_cbq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/sched/sch_cbq.c +1 -1
@@ -1614,7 +1614,7 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t err =tcf_block_get (&cl->block, &cl->filter_list, sch, extack);if (err) {kfree (cl); -return err; + goto failure; }if (tca[TCA_RATE ]) {
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 289/334 net: qualcomm: fix QCA7000 checksum handling
In reply to Greg Kroah-HartmanFrom: Stefan Wahren <[email protected]>
[ Upstream commit 429205da6c834447a57279af128bdd56ccd5225e ]
Based on tests the QCA7000 doesn't support checksum offloading. So assume ip_summed is CHECKSUM_NONE and let the kernel take care of the checksum handling. This fixes data transfer issues in noisy environments.
--- drivers/net/ethernet/qualcomm/qca_spi.c | 2 +- drivers/net/ethernet/qualcomm/qca_uart.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/qualcomm/qca_spi.c +1 -1
@@ -434,7 +434,7 @@ qcaspi_receive(struct qcaspi *qca)skb_put (qca->rx_skb, retcode); qca->rx_skb->protocol =eth_type_trans ( qca->rx_skb, qca->rx_skb->dev); - qca->rx_skb->ip_summed =CHECKSUM_UNNECESSARY ; +skb_checksum_none_assert (qca->rx_skb);netif_rx_ni (qca->rx_skb); qca->rx_skb =netdev_alloc_skb_ip_align (net_dev, net_dev->mtu +VLAN_ETH_HLEN );
drivers/net/ethernet/qualcomm/qca_uart.c +1 -1
@@ -107,7 +107,7 @@ qca_tty_receive(struct serdev_device *serdev, const unsigned char *data,skb_put (qca->rx_skb, retcode); qca->rx_skb->protocol =eth_type_trans ( qca->rx_skb, qca->rx_skb->dev); - qca->rx_skb->ip_summed =CHECKSUM_UNNECESSARY ; +skb_checksum_none_assert (qca->rx_skb);netif_rx_ni (qca->rx_skb); qca->rx_skb = netdev_alloc_skb_ip_align(netdev, netdev->mtu +
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 290/334 octeontx2-af: Fix loop in free and unmap counter
In reply to Greg Kroah-HartmanFrom: Subbaraya Sundeep <[email protected]>
[ Upstream commit 6537e96d743b89294b397b4865c6c061abae31b0 ]
When the given counter does not belong to the entry then code ends up in infinite loop because the loop cursor, entry is not getting updated further. This patch fixes that by updating entry for every iteration.
--- drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +2 -1
@@ -2990,10 +2990,11 @@ int rvu_mbox_handler_npc_mcam_unmap_counter(struct rvu *rvu, index =find_next_bit (mcam->bmap, mcam->bmap_entries, entry);if (index >= mcam->bmap_entries)break ; + entry = index +1 ; +if (mcam->entry2cntr_map[index] != req->cntr)continue ; - entry = index +1 ;npc_unmap_mcam_entry_and_cntr (rvu, mcam, blkaddr, index, req->cntr); }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 291/334 octeontx2-af: Fix mailbox errors in nix_rss_flowkey_cfg
In reply to Greg Kroah-HartmanFrom: Subbaraya Sundeep <[email protected]>
[ Upstream commit f2e4568ec95166605c77577953b2787c7f909978 ]
In npc_update_vf_flow_entry function the loop cursor 'index' is being changed inside the loop causing the loop to spin forever. This in turn hogs the kworker thread forever and no other mbox message is processed by AF driver after that. Fix this by using another variable in the loop.
--- drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +3 -3
@@ -938,7 +938,7 @@ void rvu_npc_enable_allmulti_entry(struct rvu *rvu, u16 pcifunc, int nixlf,static void npc_update_vf_flow_entry (struct rvu *rvu,struct npc_mcam *mcam,int blkaddr,u16 pcifunc,u64 rx_action) { -int actindex, index, bank; +int actindex, index, bank, entry;bool enable;if (!(pcifunc &RVU_PFVF_FUNC_MASK ))
@@ -949,7 +949,7 @@ static void npc_update_vf_flow_entry(struct rvu *rvu, struct npc_mcam *mcam,if (mcam->entry2target_pffunc[index] == pcifunc) { bank =npc_get_bank (mcam, index); actindex = index; - index &= (mcam->banksize -1 ); + entry = index & (mcam->banksize -1 );/* read vf flow entry enable status */ enable = is_mcam_entry_enabled(rvu, mcam, blkaddr,
@@ -959,7 +959,7 @@ static void npc_update_vf_flow_entry(struct rvu *rvu, struct npc_mcam *mcam, false);/* update 'action' */ rvu_write64 (rvu, blkaddr, -NPC_AF_MCAMEX_BANKX_ACTION (index, bank), +NPC_AF_MCAMEX_BANKX_ACTION (entry, bank), rx_action);if (enable) npc_enable_mcam_entry(rvu, mcam, blkaddr,
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 292/334 octeontx2-af: Fix static code analyzer reported issues
In reply to Greg Kroah-HartmanFrom: Subbaraya Sundeep <[email protected]>
[ Upstream commit 698a82ebfb4b2f2014baf31b7324b328a2a6366e ]
This patch fixes the static code analyzer reported issues in rvu_npc.c. The reported errors are different sizes of operands in bitops and returning uninitialized values.
--- drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +3 -3
@@ -23,7 +23,7 @@#define RSVD_MCAM_ENTRIES_PER_NIXLF 1/* Ucast for LFs */ #define NPC_PARSE_RESULT_DMAC_OFFSET 8 -#define NPC_HW_TSTAMP_OFFSET 8 +#define NPC_HW_TSTAMP_OFFSET 8ULL#define NPC_KEX_CHAN_MASK 0xFFFULL#define NPC_KEX_PF_FUNC_MASK 0xFFFFULL
@@ -2166,7 +2166,7 @@ static void npc_unmap_mcam_entry_and_cntr(struct rvu *rvu, int blkaddr, u16 entry, u16 cntr) { u16 index = entry & (mcam->banksize - 1); -u16 bank =npc_get_bank (mcam, entry); +u32 bank =npc_get_bank (mcam, entry);/* Remove mapping and reduce counter's refcnt */ mcam->entry2cntr_map[entry] =NPC_MCAM_INVALID_MAP ;
@@ -2788,8 +2788,8 @@ int rvu_mbox_handler_npc_mcam_shift_entry(struct rvu *rvu,struct npc_mcam *mcam = &rvu->hw->mcam;u16 pcifunc = req->hdr.pcifunc;u16 old_entry, new_entry; +int blkaddr, rc =0 ;u16 index, cntr; -int blkaddr, rc; blkaddr =rvu_get_blkaddr (rvu,BLKTYPE_NPC ,0 );if (blkaddr <0 )
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 293/334 octeontx2-af: Set proper errorcode for IPv4 checksum errors
In reply to Greg Kroah-HartmanFrom: Sunil Goutham <[email protected]>
[ Upstream commit 1e4428b6dba9b683dc2ec0a56ed7879de3200cce ]
With current config, for packets with IPv4 checksum errors, errorcode is being set to UNKNOWN. Hence added a separate errorcodes for outer and inner IPv4 checksum and changed NPC configuration accordingly.
Also turn on L2 multicast address check in NPC protocol check block.
--- drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +4 -3
@@ -2030,14 +2030,15 @@ int rvu_npc_init(struct rvu *rvu)/* Enable below for Rx pkts. * - Outer IPv4 header checksum validation. -* - Detect outer L2 broadcast address and set NPC_RESULT_S[L2M]. +* - Detect outer L2 broadcast address and set NPC_RESULT_S[L2B]. +* - Detect outer L2 multicast address and set NPC_RESULT_S[L2M]. * - Inner IPv4 header checksum validation. * - Set non zero checksum error code value */ rvu_write64 (rvu, blkaddr,NPC_AF_PCK_CFG ,rvu_read64 (rvu, blkaddr,NPC_AF_PCK_CFG ) | -BIT_ULL (32 ) |BIT_ULL (24 ) |BIT_ULL (6 ) | -BIT_ULL (2 ) |BIT_ULL (1 )); + ((u64 )NPC_EC_OIP4_CSUM <<32 ) | (NPC_EC_IIP4_CSUM <<24 ) | +BIT_ULL (7 ) |BIT_ULL (6 ) |BIT_ULL (2 ) |BIT_ULL (1 ));rvu_npc_setup_interfaces (rvu, blkaddr);
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 294/334 ipv4: fix endianness issue in inet_rtm_getroute_build_skb()
In reply to Greg Kroah-HartmanFrom: Eric Dumazet <[email protected]>
[ Upstream commit 92548b0ee220e000d81c27ac9a80e0ede895a881 ]
The UDP length field should be in network order.
This removes the following sparse error:
net/ipv4/route.c:3173:27: warning: incorrect type in assignment (different base types)net/ipv4/route.c:3173:27: expected restricted __be16 [usertype] lennet/ipv4/route.c:3173:27: got unsigned long
--- net/ipv4/route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv4/route.c +1 -1
@@ -3184,7 +3184,7 @@ static struct sk_buff *inet_rtm_getroute_build_skb(__be32 src, __be32 dst, udph =skb_put_zero (skb,sizeof (struct udphdr )); udph->source = sport; udph->dest = dport; - udph->len =sizeof (struct udphdr ); + udph->len =htons (sizeof (struct udphdr )); udph->check =0 ;break ; }
Signature · 1 line
2.30.2
Greg Kroah-Hartman
maintainerPATCH 295/334 ASoC: rt5682: Remove unused variable in rt5682_i2c_remove()
In reply to Greg Kroah-HartmanFrom: Douglas Anderson <[email protected]>
commit a1ea05723c27a6f77894a60038a7b2b12fcec9a7 upstream.
In commit 772d44526e20 ("ASoC: rt5682: Properly turn off regulators if wrong device ID") I deleted code but forgot to delete a variable that's now unused. Delete it.
--- sound/soc/codecs/rt5682-i2c.c | 2 -- 1 file changed, 2 deletions(-)
sound/soc/codecs/rt5682-i2c.c +0 -2
@@ -294,8 +294,6 @@ static void rt5682_i2c_shutdown(struct istatic int rt5682_i2c_remove (struct i2c_client *client) { -struct rt5682_priv *rt5682 =i2c_get_clientdata (client); -rt5682_i2c_shutdown (client);return 0 ;
Greg Kroah-Hartman
maintainerPATCH 296/334 iwlwifi Add support for ax201 in Samsung Galaxy Book Flex2 Alpha
In reply to Greg Kroah-HartmanFrom: Justin M. Forbes <[email protected]>
commit 2f32c147a3816d789722c0bd242a9431332ec3ed upstream.
The Samsung Galaxy Book Flex2 Alpha uses an ax201 with the ID a0f0/6074. This works fine with the existing driver once it knows to claim it. Simple patch to add the device.
--- drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/wireless/intel/iwlwifi/pcie/drv.c +1 -0
@@ -558,6 +558,7 @@ static const struct iwl_dev_info iwl_devIWL_DEV_INFO (0xA0F0 ,0x1652 , killer1650i_2ax_cfg_qu_b0_hr_b0,NULL ),IWL_DEV_INFO (0xA0F0 ,0x2074 , iwl_ax201_cfg_qu_hr,NULL ),IWL_DEV_INFO (0xA0F0 ,0x4070 , iwl_ax201_cfg_qu_hr,NULL ), +IWL_DEV_INFO (0xA0F0 ,0x6074 , iwl_ax201_cfg_qu_hr,NULL ),IWL_DEV_INFO (0x02F0 ,0x0070 , iwl_ax201_cfg_quz_hr,NULL ),IWL_DEV_INFO (0x02F0 ,0x0074 , iwl_ax201_cfg_quz_hr,NULL ), IWL_DEV_INFO(0x02F0, 0x6074, iwl_ax201_cfg_quz_hr, NULL),
Greg Kroah-Hartman
maintainerPATCH 297/334 f2fs: guarantee to write dirty data when enabling checkpoint back
In reply to Greg Kroah-HartmanFrom: Jaegeuk Kim <[email protected]>
commit dddd3d65293a52c2c3850c19b1e5115712e534d8 upstream.
We must flush all the dirty data when enabling checkpoint back. Let's guarantee that first by adding a retry logic on sync_inodes_sb(). In addition to that, this patch adds to flush data in fsync when checkpoint is disabled, which can mitigate the sync_inodes_sb() failures in advance.
--- fs/f2fs/file.c | 5 ++--- fs/f2fs/super.c | 11 ++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-)
fs/f2fs/file.c +2 -3
@@ -261,8 +261,7 @@ static int f2fs_do_sync_file(struct file };unsigned int seq_id =0 ; -if (unlikely (f2fs_readonly (inode->i_sb) || -is_sbi_flag_set (sbi,SBI_CP_DISABLED ))) +if (unlikely (f2fs_readonly (inode->i_sb)))return 0 ;trace_f2fs_sync_file_enter (inode);
@@ -276,7 +275,7 @@ static int f2fs_do_sync_file(struct file ret =file_write_and_wait_range (file, start, end);clear_inode_flag (inode,FI_NEED_IPU ); -if (ret) { +if (ret ||is_sbi_flag_set (sbi,SBI_CP_DISABLED )) {trace_f2fs_sync_file_exit (inode, cp_reason, datasync, ret);return ret; }
fs/f2fs/super.c +10 -1
@@ -2039,8 +2039,17 @@ restore_flag:static void f2fs_enable_checkpoint (struct f2fs_sb_info *sbi) { +int retry =DEFAULT_RETRY_IO_COUNT ; +/* we should flush all the data to keep data consistency */ -sync_inodes_sb (sbi->sb); +do { +sync_inodes_sb (sbi->sb); +cond_resched (); +congestion_wait (BLK_RW_ASYNC ,DEFAULT_IO_TIMEOUT ); + }while (get_pages (sbi,F2FS_DIRTY_DATA ) && retry--); + +if (unlikely (retry <0 )) +f2fs_warn (sbi,"checkpoint=enable has some unwritten data." );down_write (&sbi->gc_lock);f2fs_dirty_to_prefree (sbi);
Greg Kroah-Hartman
maintainerPATCH 298/334 time: Handle negative seconds correctly in timespec64_to_ns()
In reply to Greg Kroah-HartmanFrom: Lukas Hannen <[email protected]>
commit 39ff83f2f6cc5cc1458dfcea9697f96338210beb upstream.
timespec64_ns() prevents multiplication overflows by comparing the seconds value of the timespec to KTIME_SEC_MAX. If the value is greater or equal it returns KTIME_MAX.
But that check casts the signed seconds value to unsigned which makes the comparision true for all negative values and therefore return wrongly KTIME_MAX.
Negative second values are perfectly valid and required in some places, e.g. ptp_clock_adjtime().
Remove the cast and add a check for the negative boundary which is required to prevent undefined behaviour due to multiplication underflow.
--- include/linux/time64.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
include/linux/time64.h +7 -2
@@ -25,7 +25,9 @@ struct itimerspec64 {#define TIME64_MIN (-TIME64_MAX - 1)#define KTIME_MAX ((s64)~((u64)1 << 63)) +#define KTIME_MIN (-KTIME_MAX - 1)#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#define KTIME_SEC_MIN (KTIME_MIN / NSEC_PER_SEC)/* * Limits for settimeofday():
@@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett*/ static inline s64 timespec64_to_ns (const struct timespec64 *ts) { -/* Prevent multiplication overflow */ -if ((unsigned long long )ts->tv_sec >=KTIME_SEC_MAX ) +/* Prevent multiplication overflow / underflow */ +if (ts->tv_sec >=KTIME_SEC_MAX )return KTIME_MAX ; +if (ts->tv_sec <=KTIME_SEC_MIN ) +return KTIME_MIN ; +return ((s64 ) ts->tv_sec *NSEC_PER_SEC ) + ts->tv_nsec; }
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 5 lines · Greg Kroah-Hartman
/*
* Limits forsettimeofday():include/linux/time64.h +0
-0@@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett*/ static inline s64 timespec64_to_ns (const struct timespec64 *ts)
include/linux/time64.h static inline bool timespec64_valid_sett · line 135
Quoted 10 lines
include/linux/time64.h +5
-2{ -/* Prevent multiplication overflow */ -if ((unsigned long long )ts->tv_sec >=KTIME_SEC_MAX ) +/* Prevent multiplication overflow / underflow */ +if (ts->tv_sec >=KTIME_SEC_MAX )return KTIME_MAX ; +if (ts->tv_sec <=KTIME_SEC_MIN ) +return KTIME_MIN ; +
I just saw this get merged for the stable kernels, and had not seen this when Thomas originally merged it.
I can see how this helps the ptp_clock_adjtime() users, but I just double-checked
what other callers exist, and I think it introduces a regression in setitimer(), which does
nval = timespec64_to_ns(&value->it_value);
ninterval = timespec64_to_ns(&value->it_interval);without any further range checking that I could find. Setting timers with negative intervals sounds like a bad idea, and interpreting negative it_value as a past time instead of KTIME_SEC_MAX sounds like an unintended interface change.
I haven't done any proper analysis of the changes, so maybe it's all good, but I think we need to double-check this, and possibly revert it from the stable kernels until a final conclusion.
Arnd
Greg Kroah-Hartman
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 29 lines · Arnd Bergmann
Quoted 5 lines · Greg Kroah-Hartman
/*
* Limits forsettimeofday():include/linux/time64.h +0
-0@@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett*/ static inline s64 timespec64_to_ns (const struct timespec64 *ts)include/linux/time64.h static inline bool timespec64_valid_sett · line 135
Quoted 10 lines
include/linux/time64.h +5
-2{ -/* Prevent multiplication overflow */ -if ((unsigned long long )ts->tv_sec >=KTIME_SEC_MAX ) +/* Prevent multiplication overflow / underflow */ +if (ts->tv_sec >=KTIME_SEC_MAX )return KTIME_MAX ; +if (ts->tv_sec <=KTIME_SEC_MIN ) +return KTIME_MIN ; +I just saw this get merged for the stable kernels, and had not seen this when Thomas originally merged it.
I can see how this helps the
ptp_clock_adjtime()users, but I just double-checked
what other callers exist, and I think it introduces a regression insetitimer(), which doesnval = timespec64_to_ns(&value->it_value); ninterval = timespec64_to_ns(&value->it_interval);
Quoted 8 lines
without any further range checking that I could find. Setting timers with negative intervals sounds like a bad idea, and interpreting negative
it_valueas a past time instead ofKTIME_SEC_MAXsounds like an unintended interface change.I haven't done any proper analysis of the changes, so maybe it's all good, but I think we need to double-check this, and possibly revert it from the stable kernels until a final conclusion.
I will revert it now from all stable kernels, thanks.
greg k-h
A message that is not in the archive (sent off-list or not yet received): AM0PR01MB5410E0963A0AF9A525509DB2EEDC9@AM0PR01MB5410.eurprd01.prod.exchangelabs.com
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 11 lines · OPENSOURCE Lukas Hannen
Quoted 10 lines
I can see how this helps the
ptp_clock_adjtime()users, but I just double-checked what other callers exist, and I think it introduces a regression insetitimer(), which doesnval = timespec64_to_ns(&value->it_value); ninterval = timespec64_to_ns(&value->it_interval);without any further range checking that I could find. Setting timers with negative intervals sounds like a bad idea, and interpreting negative
it_valueas a past time instead ofKTIME_SEC_MAXsounds like an unintended interface change.
Quoted 6 lines
Hello Arnd,
I have looked into this, and it seems like before your
commitbd40a175769d("y2038: itimer: change implementation to timespec64") the "clamping and converting to positive ns" was done usingtimeval_to_ktime()andktime_to_ns().
Actually, looking back at this change, I see that there was an explicit timeval_valid()
check in get_itimerval(), and this was moved around but is still there, I guess we're
good for this syscall, and the user-visible behavior never actually changed.
Quoted 4 lines
When Commit
c5021b2547ad( "time: Prevent undefined behaviour intimespec64_to_ns()" ) put this functionally intotimespec64_to_ns(), the patchnotes mentioned the clamping toKTIME_SEC_MAX, but did not mention the explicit need to returnKTIME_SEC_MAXfor any negative input.
Right.
Quoted 5 lines
Since
timespec64_to_ns()is widely used in drivers, where negative nanosecond values are quite sensible, I propose to view both of the effects I mentioned above as separate functionalities,either to be implemented as separate functions in
time64.h(named, for example,timespec64_to_ns()andtimespec64_to_positive_ns),
I don't mind having the common version work the way it does after your patch, I was only worried about silently changing the behavior for a documented syscall.
Quoted 7 lines
or alternatively, since the
setitimer()code seems to be the only one not expecting negative nanoseconds out oftimespec64_to_ns()when fed negative input, the clamping of negative nanosecond values toKTIME_SEC_MAXto be moved into thesetitimer()code, andtimespec64_to_ns()to be changed according to the patch I submitted.Both of those alternatives seem trivial and I can send in patches for both of them, but since this is more a matter of style I would like to hear your opinions on this beforehand.
It looks like we don't have to do anything for setitimer(), but that was just the first one that
I happened to look at. Did you check the other instances to see if anything might be going
wrong there? If they are all good, then I have no other concerns and we should probably
put your fix back into the stable kernels (Greg has just reverted it after my initial mail).
I went through all instances other than the ptp related ones, and I'm pretty confident
that they are all good now, in each case either your patch fixes a bug or the value is
already known to be positive and it doesn't matter. Are you confident that the ptp
instances are all good as well?
I did stumble over one small detail:
if (ts->tv_sec <= KTIME_SEC_MIN)
return KTIME_MIN;I think this is not entirely correct for the case of tv_sec==KTIME_SEC_MIN with a nonzero tv_nsec, as we now round down to the full second. Not sure if that's worth changing, as we also round up for any value between KTIME_SEC_MAX*NSEC_PER_SEC and KTIME_MAX, or betweenKTIME_MIN and KTIME_SEC_MIN*NSEC_PER_SEC.
In practice I guess we care very little about the last nanosecond in the corner cases.
Arnd
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 7 lines · Arnd Bergmann
On Thu, Sep 16, 2021 at 6:50 PM OPENSOURCE Lukas Hannen <[email protected]> wrote:
I did stumble over one small detail:if (ts->tv_sec <= KTIME_SEC_MIN) return KTIME_MIN;
Quoted 7 lines
I think this is not entirely correct for the case of tv_sec==KTIME_SEC_MIN with a nonzero
tv_nsec, as we now round down to the full second. Not sure if that's worth changing, as we also round up for any value between KTIME_SEC_MAX*NSEC_PER_SEC andKTIME_MAX, or betweenKTIME_MINand KTIME_SEC_MIN*NSEC_PER_SEC.
In practice I guess we care very little about the last nanosecond in the corner cases.
It's completely irrelevant whether the result is off by one second related to the 292 years limit.
Thanks,
tglx
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
In reply to Arnd BergmannArnd,
Quoted 29 lines · Arnd Bergmann
Quoted 5 lines · Greg Kroah-Hartman
/*
* Limits forsettimeofday():include/linux/time64.h +0
-0@@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett*/ static inline s64 timespec64_to_ns (const struct timespec64 *ts)include/linux/time64.h static inline bool timespec64_valid_sett · line 135
Quoted 10 lines
include/linux/time64.h +5
-2{ -/* Prevent multiplication overflow */ -if ((unsigned long long )ts->tv_sec >=KTIME_SEC_MAX ) +/* Prevent multiplication overflow / underflow */ +if (ts->tv_sec >=KTIME_SEC_MAX )return KTIME_MAX ; +if (ts->tv_sec <=KTIME_SEC_MIN ) +return KTIME_MIN ; +I just saw this get merged for the stable kernels, and had not seen this when Thomas originally merged it.
I can see how this helps the
ptp_clock_adjtime()users, but I just double-checked
what other callers exist, and I think it introduces a regression insetitimer(), which doesnval = timespec64_to_ns(&value->it_value); ninterval = timespec64_to_ns(&value->it_interval);
Quoted 8 lines
without any further range checking that I could find. Setting timers with negative intervals sounds like a bad idea, and interpreting negative
it_valueas a past time instead ofKTIME_SEC_MAXsounds like an unintended interface change.I haven't done any proper analysis of the changes, so maybe it's all good, but I think we need to double-check this, and possibly revert it from the stable kernels until a final conclusion.
I have done the analysis. setitimer() does not have any problem with that simply because it already checks at the call site that the seconds value is > 0 and so do all the other user visible interfaces. See get_itimerval() ...
Granted that the kernel internal interfaces do not have those checks, but they already have other safety nets in place to prevent this and I could not identify any callsite which has trouble with that change.
If I failed to spot one then what the heck is the problem? It was broken before that change already!
I usually spend quite some time on tagging patches for stable and it's annoying me that this patch got reverted while stuff which I explicitely did not tag for stable got backported for whatever reason and completely against the stable rules:
627ef5ae2df8 ("hrtimer: Avoid double reprogramming in __hrtimer_start_range_ns()")
What the heck qualifies this to be backported?
1) It's hot of the press and just got merged in the 5.15-rc1 merge window and is not tagged for stable
2) https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
clearly states the rules but obviously our new fangled "AI" driven approach to select patches for stable is blissfully ignorant of these rules. I assume that AI stands for "Artifical Ignorance' here.
I already got a private bug report vs. that on 5.10.65. Annoyingly 5.10.5 does not have the issue despite the fact that the resulting diff between those two versions in hrtimer.c is just in comments.
Bah!
Thanks,
tglx
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 12 lines · Thomas Gleixner
I usually spend quite some time on tagging patches for stable and it's annoying me that this patch got reverted while stuff which I explicitely did not tag for stable got backported for whatever reason and completely against the stable rules:
627ef5ae2df8("hrtimer: Avoid double reprogramming in__hrtimer_start_range_ns()")What the heck qualifies this to be backported?
1) It's hot of the press and just got merged in the 5.15-rc1 merge window and is not tagged for stable
Quoted 8 lines
2) https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
clearly states the rules but obviously our new fangled "AI" driven approach to select patches for stable is blissfully ignorant of these rules. I assume that AI stands for "Artifical Ignorance' here.
I already got a private bug report vs. that on 5.10.65. Annoyingly 5.10.5 does not have the issue despite the fact that the resulting diff
5.14.5 obviously...
Quoted 7 lines
between those two versions in
hrtimer.cis just in comments.Bah!
Thanks,
tglx
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
In reply to Thomas GleixnerQuoted 6 lines · Thomas Gleixner
On Wed, Sep 15 2021 at 21:00, Arnd Bergmann wrote:
I have done the analysis.
setitimer()does not have any problem with that simply because it already checks at the call site that the seconds value is > 0 and so do all the other user visible interfaces. Seeget_itimerval()...
Right, I now came to the same conclusion after taking a closer look, see my reply from yesterday.
Quoted 6 lines
Granted that the kernel internal interfaces do not have those checks, but they already have other safety nets in place to prevent this and I could not identify any callsite which has trouble with that change.
If I failed to spot one then what the heck is the problem? It was broken before that change already!
My bad for the unfortunate timing. When only saw the patch when Greg posted it during the stable review and wasn't completely sure about it at the time, so I was hoping that he could just hold off until you had a chance to reply either saying that you had already checked this case or that it was dangerous, but now it's already reverted.
I agree we should put back the fix into all stable kernels.
Arnd
Greg Kroah-Hartman
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 18 lines · Arnd Bergmann
Quoted 6 lines · Thomas Gleixner
On Wed, Sep 15 2021 at 21:00, Arnd Bergmann wrote:
I have done the analysis.
setitimer()does not have any problem with that simply because it already checks at the call site that the seconds value is > 0 and so do all the other user visible interfaces. Seeget_itimerval()...Right, I now came to the same conclusion after taking a closer look, see my reply from yesterday.
Quoted 6 lines
Granted that the kernel internal interfaces do not have those checks, but they already have other safety nets in place to prevent this and I could not identify any callsite which has trouble with that change.
If I failed to spot one then what the heck is the problem? It was broken before that change already!
Quoted 7 lines
My bad for the unfortunate timing. When only saw the patch when Greg posted it during the stable review and wasn't completely sure about it at the time, so I was hoping that he could just hold off until you had a chance to reply either saying that you had already checked this case or that it was dangerous, but now it's already reverted.
I agree we should put back the fix into all stable kernels.
Ok, I'll queue it up again after this round goes out.
thanks for the additional review.
greg k-h
Greg Kroah-Hartman
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
In reply to Thomas GleixnerQuoted 67 lines · Thomas Gleixner
Arnd,
Quoted 29 lines · Arnd Bergmann
Quoted 5 lines · Greg Kroah-Hartman
/*
* Limits forsettimeofday():include/linux/time64.h +0
-0@@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett*/ static inline s64 timespec64_to_ns (const struct timespec64 *ts)include/linux/time64.h static inline bool timespec64_valid_sett · line 135
Quoted 10 lines
include/linux/time64.h +5
-2{ -/* Prevent multiplication overflow */ -if ((unsigned long long )ts->tv_sec >=KTIME_SEC_MAX ) +/* Prevent multiplication overflow / underflow */ +if (ts->tv_sec >=KTIME_SEC_MAX )return KTIME_MAX ; +if (ts->tv_sec <=KTIME_SEC_MIN ) +return KTIME_MIN ; +I just saw this get merged for the stable kernels, and had not seen this when Thomas originally merged it.
I can see how this helps the
ptp_clock_adjtime()users, but I just double-checked
what other callers exist, and I think it introduces a regression insetitimer(), which doesnval = timespec64_to_ns(&value->it_value); ninterval = timespec64_to_ns(&value->it_interval);Quoted 8 lines
without any further range checking that I could find. Setting timers with negative intervals sounds like a bad idea, and interpreting negative
it_valueas a past time instead ofKTIME_SEC_MAXsounds like an unintended interface change.I haven't done any proper analysis of the changes, so maybe it's all good, but I think we need to double-check this, and possibly revert it from the stable kernels until a final conclusion.
I have done the analysis.
setitimer()does not have any problem with that simply because it already checks at the call site that the seconds value is > 0 and so do all the other user visible interfaces. Seeget_itimerval()...Granted that the kernel internal interfaces do not have those checks, but they already have other safety nets in place to prevent this and I could not identify any callsite which has trouble with that change.
If I failed to spot one then what the heck is the problem? It was broken before that change already!
I usually spend quite some time on tagging patches for stable and it's annoying me that this patch got reverted while stuff which I explicitely did not tag for stable got backported for whatever reason and completely against the stable rules:
627ef5ae2df8("hrtimer: Avoid double reprogramming in__hrtimer_start_range_ns()")What the heck qualifies this to be backported?
1) It's hot of the press and just got merged in the 5.15-rc1 merge window and is not tagged for stable
2) https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
Quoted 7 lines
clearly states the rules but obviously our new fangled "AI" driven approach to select patches for stable is blissfully ignorant of these rules. I assume that AI stands for "Artifical Ignorance' here.
I already got a private bug report vs. that on 5.10.65. Annoyingly 5.10.5 does not have the issue despite the fact that the resulting diff between those two versions in
hrtimer.cis just in comments.
Looks like Sasha picked it up with the AUTOSEL process, and emailed you about this on Sep 5:
https://lore.kernel.org/r/[email protected]
I will revert it if you don't think it should be in the stable trees.
Also, if you want AUTOSEL to not look at any hrtimer.c patches, just let us know and Sasha will add it to the ignore-list.
thanks,
greg k-h
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 4 lines · Greg Kroah-Hartman
Quoted 3 lines · Thomas Gleixner
I already got a private bug report vs. that on 5.10.65. Annoyingly 5.10.5 does not have the issue despite the fact that the resulting diff between those two versions in
hrtimer.cis just in comments.
The bug report turned out to be a red hering. Probably caused by a bisect gone wrong. The real culprit was the posix-cpu-timer change which got reverted already.
Quoted 3 lines
Looks like Sasha picked it up with the AUTOSEL process, and emailed you about this on Sep 5:
https://lore.kernel.org/r/[email protected]
which I obviously missed.
Quoted 1 line
I will revert it if you don't think it should be in the stable trees.
It's a pure performance improvement, so according to stable rules it should not be there.
Quoted 2 lines
Also, if you want AUTOSEL to not look at any
hrtimer.cpatches, just let us know and Sasha will add it to the ignore-list.
Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
Thanks,
tglx
Greg Kroah-Hartman
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 27 lines · Thomas Gleixner
Quoted 4 lines · Greg Kroah-Hartman
Quoted 3 lines · Thomas Gleixner
I already got a private bug report vs. that on 5.10.65. Annoyingly 5.10.5 does not have the issue despite the fact that the resulting diff between those two versions in
hrtimer.cis just in comments.The bug report turned out to be a red hering. Probably caused by a bisect gone wrong. The real culprit was the posix-cpu-timer change which got reverted already.
Quoted 3 lines
Looks like Sasha picked it up with the AUTOSEL process, and emailed you about this on Sep 5:
https://lore.kernel.org/r/[email protected]which I obviously missed.
Quoted 1 line
I will revert it if you don't think it should be in the stable trees.
It's a pure performance improvement, so according to stable rules it should not be there.
Quoted 2 lines
Also, if you want AUTOSEL to not look at any
hrtimer.cpatches, just let us know and Sasha will add it to the ignore-list.Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Quoted 7 lines
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
The whole point of the AUTOSEL patches are for the huge numbers of subsystems where maintainers and developers do not care about the stable trees at all, and so they do not mark patches to be backported. So requireing an opt-in like this would defeat the purpose.
We do allow the ability to take files/subsystems out of the AUTOSEL process as there are many maintainers that do do this right and get annoyed when patches are picked that they feel shouldn't have. That's the best thing we can do for stuff like this.
thanks,
greg k-h
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Greg,
Quoted 17 lines · Greg Kroah-Hartman
Quoted 10 lines · Thomas Gleixner
Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
The whole point of the AUTOSEL patches are for the huge numbers of subsystems where maintainers and developers do not care about the stable trees at all, and so they do not mark patches to be backported. So requireing an opt-in like this would defeat the purpose.
Quoted 4 lines
We do allow the ability to take files/subsystems out of the AUTOSEL process as there are many maintainers that do do this right and get annoyed when patches are picked that they feel shouldn't have. That's the best thing we can do for stuff like this.
I guess I was not able to express myself correctly. What I wanted to say is:
1) Default is AUTOSEL
2) Maintainer can take files/subsystems out of AUTOSEL completely
Exists today
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Is new and I would be the first to opt-in :)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Does that make more sense now?
Thanks,
tglx
Greg Kroah-Hartman
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 38 lines · Thomas Gleixner
Greg,
Quoted 17 lines · Greg Kroah-Hartman
Quoted 10 lines · Thomas Gleixner
Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
The whole point of the AUTOSEL patches are for the huge numbers of subsystems where maintainers and developers do not care about the stable trees at all, and so they do not mark patches to be backported. So requireing an opt-in like this would defeat the purpose.
Quoted 4 lines
We do allow the ability to take files/subsystems out of the AUTOSEL process as there are many maintainers that do do this right and get annoyed when patches are picked that they feel shouldn't have. That's the best thing we can do for stuff like this.
I guess I was not able to express myself correctly. What I wanted to say is:
1) Default is AUTOSEL
2) Maintainer can take files/subsystems out of AUTOSEL completely
Exists today
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Quoted 7 lines
Is new and I would be the first to opt-in :)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Does that make more sense now?
Ah, yes, that makes much more sense, sorry for the confusion.
Sasha, what do you think? You are the one that scripts all of this, not me :)
greg k-h
Sasha Levin
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 45 lines · Greg Kroah-Hartman
Quoted 44 lines · Thomas Gleixner
Greg,
Quoted 17 lines · Greg Kroah-Hartman
Quoted 10 lines · Thomas Gleixner
Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
The whole point of the AUTOSEL patches are for the huge numbers of subsystems where maintainers and developers do not care about the stable trees at all, and so they do not mark patches to be backported. So requireing an opt-in like this would defeat the purpose.
Quoted 4 lines
We do allow the ability to take files/subsystems out of the AUTOSEL process as there are many maintainers that do do this right and get annoyed when patches are picked that they feel shouldn't have. That's the best thing we can do for stuff like this.
I guess I was not able to express myself correctly. What I wanted to say is:
1) Default is AUTOSEL
2) Maintainer can take files/subsystems out of AUTOSEL completely
Exists today
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Is new and I would be the first to opt-in :)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Quoted 6 lines
Quoted 1 line
Does that make more sense now?
Ah, yes, that makes much more sense, sorry for the confusion.
Sasha, what do you think? You are the one that scripts all of this, not me :)
I could give it a go. It adds some complexity here but is probably worth it to avoid issues.
Let me think about the best way to go about it.
Signature · 2 lines
Thanks, Sasha
Sasha Levin
maintainerRe: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 50 lines · Sasha Levin
Quoted 49 lines · Greg Kroah-Hartman
Quoted 38 lines · Thomas Gleixner
Greg,
Quoted 17 lines · Greg Kroah-Hartman
Quoted 10 lines · Thomas Gleixner
Nah. I try to pay more attention. I'm not against AUTOSEL per se, but could we change the rules slightly?
Any change which is selected by AUTOSEL and lacks a Cc: stable@... is put on hold until acked by the maintainer unless it is a prerequisite for applying a stable tagged fix?
This can be default off and made effective on maintainer request.
Hmm?
The whole point of the AUTOSEL patches are for the huge numbers of subsystems where maintainers and developers do not care about the stable trees at all, and so they do not mark patches to be backported. So requireing an opt-in like this would defeat the purpose.
Quoted 4 lines
We do allow the ability to take files/subsystems out of the AUTOSEL process as there are many maintainers that do do this right and get annoyed when patches are picked that they feel shouldn't have. That's the best thing we can do for stuff like this.
I guess I was not able to express myself correctly. What I wanted to say is:
1) Default is AUTOSEL
2) Maintainer can take files/subsystems out of AUTOSEL completely
Exists today
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Quoted 7 lines
Is new and I would be the first to opt-in :)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Does that make more sense now?
Ah, yes, that makes much more sense, sorry for the confusion.
Quoted 7 lines
Quoted 2 lines
Sasha, what do you think? You are the one that scripts all of this, not me :)
I could give it a go. It adds some complexity here but is probably worth it to avoid issues.
Let me think about the best way to go about it.
So I'm thinking of yet another patch series that would go out, but instead of AUTOSEL it'll be tagged with "MANUALSEL". It would work the exact same way as AUTOSEL, without the final step of queueing up the commits into the stable trees.
Thomas, do you want to give it a go? Want to describe how I filter for commits you'd be taking care of? In the past I'd grep a combo of paths and committers (i.e. net/ && davem@), but you have your hands in too many things :)
Signature · 2 lines
Thanks, Sasha
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 40 lines · Sasha Levin
Quoted 26 lines · Sasha Levin
Quoted 25 lines · Greg Kroah-Hartman
Quoted 14 lines · Thomas Gleixner
I guess I was not able to express myself correctly. What I wanted to say is:
1) Default is AUTOSEL
2) Maintainer can take files/subsystems out of AUTOSEL completely
Exists today
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Quoted 7 lines
Is new and I would be the first to opt-in :)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Does that make more sense now?
Ah, yes, that makes much more sense, sorry for the confusion.
Quoted 7 lines
Quoted 2 lines
Sasha, what do you think? You are the one that scripts all of this, not me :)
I could give it a go. It adds some complexity here but is probably worth it to avoid issues.
Let me think about the best way to go about it.
So I'm thinking of yet another patch series that would go out, but instead of AUTOSEL it'll be tagged with "MANUALSEL". It would work the exact same way as AUTOSEL, without the final step of queueing up the commits into the stable trees.
Quoted 4 lines
Thomas, do you want to give it a go? Want to describe how I filter for commits you'd be taking care of? In the past I'd grep a combo of paths and committers (i.e.
net/&& davem@), but you have your hands in too many things :)
Indeed. :(
So pretty much all what matches in MAINTAINERS entries where my name happened to end up for some reasons. That would be a good start.
Might be a bit overbroad as it also includes x86/kvm, x86/xen, x86/pci which I'm not that involved with, but to make it simple for you, I just volunteered the relevant maintainers (CCed) to participate in that experiment. :)
Thanks,
tglx
Re: [PATCH 5.14 298/334] time: Handle negative seconds correctly in timespec64_to_ns()
Quoted 11 lines · Thomas Gleixner
3) Maintainer allows AUTOSEL, but anything picked from files/subsystems without a stable tag requires an explicit ACK from the maintainer for the backport.
Is new and I would be the first to opt-in:)
My rationale for #3 is that even when being careful about stable tags, it happens that one is missing. Occasionaly AUTOSEL finds one of those in my subsystems which I appreciate.
Does that make more sense now?
I like this!
Quoted 4 lines
Might be a bit overbroad as it also includes x86/kvm, x86/xen, x86/pci which I'm not that involved with, but to make it simple for you, I just volunteered the relevant maintainers (CCed) to participate in that experiment.
I would opt in to MANUALSEL too; so Sasha, feel free to do the same for everything that I'm involved in by the MAINTAINERS file, or we can start with x86/kvm via the generic x86 entries.
Thanks,
Paolo
Greg Kroah-Hartman
maintainerPATCH 299/334 auxdisplay: hd44780: Fix oops on module unloading
In reply to Greg Kroah-HartmanFrom: Lars Poeschel <[email protected]>
commit 333ff32d54cdefc2e479892e7f15ac91e026b57d upstream.
--- drivers/auxdisplay/hd44780.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/auxdisplay/hd44780.c +1 -1
@@ -323,8 +323,8 @@ static int hd44780_remove(struct platfor {struct charlcd *lcd =platform_get_drvdata (pdev); -kfree (lcd->drvdata);charlcd_unregister (lcd); +kfree (lcd->drvdata);kfree (lcd);return 0 ;
Greg Kroah-Hartman
maintainerPATCH 300/334 io_uring: limit fixed table size by RLIMIT_NOFILE
In reply to Greg Kroah-HartmanFrom: Pavel Begunkov <[email protected]>
commit 3a1b8a4e843f96b636431450d8d79061605cf74b upstream.
Limit the number of files in io_uring fixed tables by RLIMIT_NOFILE, that's the first and the simpliest restriction that we should impose.
--- fs/io_uring.c | 2 ++ 1 file changed, 2 insertions(+)
fs/io_uring.c +2 -0
@@ -7722,6 +7722,8 @@ static int io_sqe_files_register(structreturn -EINVAL ;if (nr_args >IORING_MAX_FIXED_FILES )return -EMFILE ; +if (nr_args >rlimit (RLIMIT_NOFILE )) +return -EMFILE ; ret =io_rsrc_node_switch_start (ctx);if (ret)return ret;
Greg Kroah-Hartman
maintainerPATCH 301/334 io_uring: IORING_OP_WRITE needs hash_reg_file set
In reply to Greg Kroah-HartmanFrom: Jens Axboe <[email protected]>
commit 7b3188e7ed54102a5dcc73d07727f41fb528f7c8 upstream.
During some testing, it became evident that using IORING_OP_WRITE doesn't hash buffered writes like the other writes commands do. That's simply an oversight, and can cause performance regressions when doing buffered writes with this command.
Correct that and add the flag, so that buffered writes are correctly hashed when using the non-iovec based write command.
--- fs/io_uring.c | 1 + 1 file changed, 1 insertion(+)
fs/io_uring.c +1 -0
@@ -1001,6 +1001,7 @@ static const struct io_op_def io_op_defs }, [IORING_OP_WRITE] = { .needs_file = 1, + .hash_reg_file = 1, .unbound_nonreg_file = 1, .pollout = 1, .plug = 1,
Greg Kroah-Hartman
maintainerPATCH 302/334 io_uring: io_uring_complete() trace should take an integer
In reply to Greg Kroah-HartmanFrom: Jens Axboe <[email protected]>
commit 2fc2a7a62eb58650e71b4550cf6fa6cc0a75b2d2 upstream.
It currently takes a long, and while that's normally OK, the io_uring limit is an int. Internally in io_uring it's an int, but sometimes it's passed as a long. That can yield confusing results where a completions seems to generate a huge result:
ou-sqp-1297-1298 [001] ...1 788.056371: io_uring_complete: ring 000000000e98e046, user_data 0x0, result 4294967171, cflags 0
which is due to -ECANCELED being stored in an unsigned, and then passed in as a long. Using the right int type, the trace looks correct:
iou-sqp-338-339 [002] ...1 15.633098: io_uring_complete: ring 00000000e0ac60cf, user_data 0x0, result -125, cflags 0
--- include/trace/events/io_uring.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/trace/events/io_uring.h +3 -3
@@ -295,14 +295,14 @@ TRACE_EVENT(io_uring_fail_link,*/ TRACE_EVENT(io_uring_complete, - TP_PROTO(void *ctx, u64 user_data, long res, unsigned cflags), + TP_PROTO(void *ctx, u64 user_data, int res, unsigned cflags), TP_ARGS(ctx, user_data, res, cflags), TP_STRUCT__entry ( __field( void *, ctx ) __field( u64, user_data ) - __field( long, res ) + __field( int, res ) __field( unsigned, cflags ) ),
@@ -313,7 +313,7 @@ TRACE_EVENT(io_uring_complete, __entry->cflags = cflags; ), -TP_printk ("ring %p, user_data 0x%llx, result %ld, cflags %x" , +TP_printk ("ring %p, user_data 0x%llx, result %d, cflags %x" , __entry->ctx, (unsigned long long )__entry->user_data, __entry->res, __entry->cflags) );
Greg Kroah-Hartman
maintainerPATCH 303/334 io_uring: fail links of cancelled timeouts
In reply to Greg Kroah-HartmanFrom: Pavel Begunkov <[email protected]>
commit 2ae2eb9dde18979b40629dd413b9adbd6c894cdf upstream.
When we cancel a timeout we should mark it with REQ_F_FAIL, so linked requests are cancelled as well, but not queued for further execution.
--- fs/io_uring.c | 2 ++ 1 file changed, 2 insertions(+)
fs/io_uring.c +2 -0
@@ -1329,6 +1329,8 @@ static void io_kill_timeout(struct io_kistruct io_timeout_data *io = req->async_data;if (hrtimer_try_to_cancel (&io->timer) !=-1 ) { +if (status) +req_set_fail (req);atomic_set (&req->ctx->cq_timeouts,atomic_read (&req->ctx->cq_timeouts) +1 );list_del_init (&req->timeout.list);
Greg Kroah-Hartman
maintainerPATCH 304/334 bio: fix page leak bio_add_hw_page failure
In reply to Greg Kroah-HartmanFrom: Pavel Begunkov <[email protected]>
commit d9cf3bd531844ffbfe94b16e417037a16efc988d upstream.
__bio_iov_append_get_pages() doesn't put not appended pages on bio_add_hw_page() failure, so potentially leaking them, fix it. Also, do the same for __bio_iov_iter_get_pages(), even though it looks like it can't be triggered by userspace in this case.
--- block/bio.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
block/bio.c +13 -2
@@ -979,6 +979,14 @@ static int bio_iov_bvec_set_append(strucreturn 0 ; } +static void bio_put_pages (struct page **pages,size_t size,size_t off) +{ +size_t i, nr =DIV_ROUND_UP (size + (off & ~PAGE_MASK ),PAGE_SIZE ); + +for (i =0 ; i < nr; i++) +put_page (pages[i]); +} +#define PAGE_PTRS_PER_BVEC (sizeof(struct bio_vec) / sizeof(struct page *))/**
@@ -1023,8 +1031,10 @@ static int __bio_iov_iter_get_pages(struif (same_page)put_page (page); }else { -if (WARN_ON_ONCE (bio_full (bio, len))) -return -EINVAL ; +if (WARN_ON_ONCE (bio_full (bio, len))) { +bio_put_pages (pages + i, left, offset); +return -EINVAL ; + }__bio_add_page (bio, page, len, offset); } offset =0 ;
@@ -1069,6 +1079,7 @@ static int __bio_iov_append_get_pages(st len =min_t (size_t,PAGE_SIZE - offset, left);if (bio_add_hw_page (q, bio, page, len, offset, max_append_sectors, &same_page) != len) { +bio_put_pages (pages + i, left, offset); ret = -EINVAL ;break ; }
Greg Kroah-Hartman
maintainerPATCH 305/334 raid1: ensure write behind bio has less than BIO_MAX_VECS sectors
In reply to Greg Kroah-HartmanFrom: Guoqing Jiang <[email protected]>
commit 6607cd319b6b91bff94e90f798a61c031650b514 upstream.
We can't split write behind bio with more than BIO_MAX_VECS sectors, otherwise the below call trace was triggered because we could allocate oversized write behind bio later.
[ 8.097936] bvec_alloc+0x90/0xc0 [ 8.098934] bio_alloc_bioset+0x1b3/0x260 [ 8.099959] raid1_make_request+0x9ce/0xc50 [raid1] [ 8.100988] ? __bio_clone_fast+0xa8/0xe0 [ 8.102008] md_handle_request+0x158/0x1d0 [md_mod] [ 8.103050] md_submit_bio+0xcd/0x110 [md_mod] [ 8.104084] submit_bio_noacct+0x139/0x530 [ 8.105127] submit_bio+0x78/0x1d0 [ 8.106163] ext4_io_submit+0x48/0x60 [ext4] [ 8.107242] ext4_writepages+0x652/0x1170 [ext4] [ 8.108300] ? do_writepages+0x41/0x100 [ 8.109338] ? __ext4_mark_inode_dirty+0x240/0x240 [ext4] [ 8.110406] do_writepages+0x41/0x100 [ 8.111450] __filemap_fdatawrite_range+0xc5/0x100 [ 8.112513] file_write_and_wait_range+0x61/0xb0 [ 8.113564] ext4_sync_file+0x73/0x370 [ext4] [ 8.114607] __x64_sys_fsync+0x33/0x60 [ 8.115635] do_syscall_64+0x33/0x40 [ 8.116670] entry_SYSCALL_64_after_hwframe+0x44/0xae
Thanks for the comment from Christoph.
[1]. https://bugs.archlinux.org/task/70992
--- drivers/md/raid1.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/md/raid1.c +19 -0
@@ -1329,6 +1329,7 @@ static void raid1_write_request(struct mstruct raid1_plug_cb *plug =NULL ;int first_clone;int max_sectors; +bool write_behind = false;if (mddev_is_clustered (mddev) && md_cluster_ops->area_resyncing(mddev, WRITE,
@@ -1381,6 +1382,15 @@ static void raid1_write_request(struct m max_sectors = r1_bio->sectors;for (i =0 ; i < disks; i++) {struct md_rdev *rdev =rcu_dereference (conf->mirrors[i].rdev); + +/* +* The write-behind io is only attempted on drives marked as +* write-mostly, which means we could allocate write behind +* bio later. +*/ +if (rdev &&test_bit (WriteMostly, &rdev->flags)) + write_behind = true; +if (rdev &&unlikely (test_bit (Blocked, &rdev->flags))) {atomic_inc (&rdev->nr_pending); blocked_rdev = rdev;
@@ -1454,6 +1464,15 @@ static void raid1_write_request(struct m goto retry_write; } +/* +* When using a bitmap, we may call alloc_behind_master_bio below. +* alloc_behind_master_bio allocates a copy of the data payload a page +* at a time and thus needs a new bio that can fit the whole payload +* this bio in page sized chunks. +*/ +if (write_behind && bitmap) + max_sectors =min_t (int, max_sectors, +BIO_MAX_VECS * (PAGE_SIZE >>9 ));if (max_sectors <bio_sectors (bio)) {struct bio *split =bio_split (bio, max_sectors,GFP_NOIO , &conf->bio_split);
Greg Kroah-Hartman
maintainerPATCH 306/334 cifs: Do not leak EDEADLK to dgetents64 for STATUS_USER_SESSION_DELETED
In reply to Greg Kroah-HartmanFrom: Ronnie Sahlberg <[email protected]>
commit 3998f0b8bc49ec784990971dc1f16bf367b19078 upstream.
RHBZ: 1994393
If we hit a STATUS_USER_SESSION_DELETED for the Create part in the Create/QueryDirectory compound that starts a directory scan
we will leak EDEADLK back to userspace and surprise glibc and the application.
Pick this up initiate_cifs_search() and retry a small number of tries before we return an error to userspace.
--- fs/cifs/readdir.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
fs/cifs/readdir.c +22 -1
@@ -369,7 +369,7 @@ int get_symlink_reparse_path(char *full_*/ static int -initiate_cifs_search (const unsigned int xid,struct file *file, +_initiate_cifs_search (const unsigned int xid,struct file *file,const char *full_path) {__u16 search_flags;
@@ -451,6 +451,27 @@ error_exit:return rc; } +static int +initiate_cifs_search (const unsigned int xid,struct file *file, +const char *full_path) +{ +int rc, retry_count =0 ; + +do { + rc =_initiate_cifs_search (xid, file, full_path); +/* +* If we don't have enough credits to start reading the +* directory just try again after short wait. +*/ +if (rc != -EDEADLK ) +break ; + +usleep_range (512 ,2048 ); + }while (retry_count++ <5 ); + +return rc; +} +/* return length of unicode string in bytes */ static int cifs_unicode_bytelen (const char *str) {
Greg Kroah-Hartman
maintainerPATCH 307/334 smb3: fix posix extensions mount option
In reply to Greg Kroah-HartmanFrom: Steve French <[email protected]>
commit 7321be2663da5922343cc121f1ff04924cee2e76 upstream.
We were incorrectly initializing the posix extensions in the conversion to the new mount API.
--- fs/cifs/fs_context.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
fs/cifs/fs_context.c +9 -2
@@ -1266,10 +1266,17 @@ static int smb3_fs_context_parse_param(s ctx->posix_paths =1 ;break ;case Opt_unix: -if (result.negated) +if (result.negated) { +if (ctx->linux_ext ==1 ) +pr_warn_once ("conflicting posix mount options specified\n" ); ctx->linux_ext =0 ; -else ctx->no_linux_ext =1 ; + }else { +if (ctx->no_linux_ext ==1 ) +pr_warn_once ("conflicting posix mount options specified\n" ); + ctx->linux_ext =1 ; + ctx->no_linux_ext =0 ; + }break ;case Opt_nocase: ctx->nocase =1 ;
Greg Kroah-Hartman
maintainerPATCH 308/334 tty: Fix data race between tiocsti() and flush_to_ldisc()
In reply to Greg Kroah-HartmanFrom: Nguyen Dinh Phi <[email protected]>
commit bb2853a6a421a052268eee00fd5d3f6b3504b2b1 upstream.
The ops->receive_buf() may be accessed concurrently from these two functions. If the driver flushes data to the line discipline receive_buf() method while tiocsti() is waiting for the ops->receive_buf() to finish its work, the data race will happen.
For example:tty_ioctl |tty_ldisc_receive_buf
->tioctsi | ->tty_port_default_receive_buf
| ->tty_ldisc_receive_buf
->hci_uart_tty_receive | ->hci_uart_tty_receive
->h4_recv | ->h4_recvIn this case, the h4 receive buffer will be overwritten by the latecomer, and we will lost the data.
Hence, change tioctsi() function to use the exclusive lock interface from tty_buffer to avoid the data race.
--- drivers/tty/tty_io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/tty/tty_io.c +2 -2
@@ -2290,8 +2290,6 @@ static int tty_fasync(int fd, struct fil* Locking: * Called functions take tty_ldiscs_lock * current->signal->tty check is safe without locks -* -* FIXME: may race normal receive processing */ static int tiocsti (struct tty_struct *tty,char __user *p)
@@ -2307,8 +2305,10 @@ static int tiocsti(struct tty_struct *tt ld =tty_ldisc_ref_wait (tty);if (!ld)return -EIO ; +tty_buffer_lock_exclusive (tty->port);if (ld->ops->receive_buf) ld->ops->receive_buf (tty, &ch, &mbz,1 ); +tty_buffer_unlock_exclusive (tty->port);tty_ldisc_deref (ld);return 0 ; }
Greg Kroah-Hartman
maintainerPATCH 309/334 perf/x86/intel/uncore: Fix IIO cleanup mapping procedure for SNR/ICX
In reply to Greg Kroah-HartmanFrom: Alexander Antonov <[email protected]>
commit 3f2cbe3810a60111a33f5f6267bd5a237b826fc9 upstream.
skx_iio_cleanup_mapping() is re-used for snr and icx, but in those cases it fails to use the appropriate XXX_iio_mapping_group and as such fails to free previously allocated resources, leading to memory leaks.
--- arch/x86/events/intel/uncore_snbep.c | 40 ++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 12 deletions(-)
arch/x86/events/intel/uncore_snbep.c +28 -12
@@ -3838,26 +3838,32 @@ clear_attr_update:return ret; } -static int skx_iio_set_mapping (struct intel_uncore_type *type) -{ -return pmu_iio_set_mapping (type, &skx_iio_mapping_group); -} - -static void skx_iio_cleanup_mapping (struct intel_uncore_type *type) +static void +pmu_iio_cleanup_mapping (struct intel_uncore_type *type,struct attribute_group *ag) { -struct attribute **attr = skx_iio_mapping_group.attrs; +struct attribute **attr = ag->attrs;if (!attr)return ;for (; *attr; attr++)kfree ((*attr)->name); -kfree (attr_to_ext_attr (*skx_iio_mapping_group.attrs)); -kfree (skx_iio_mapping_group.attrs); - skx_iio_mapping_group.attrs =NULL ; +kfree (attr_to_ext_attr (*ag->attrs)); +kfree (ag->attrs); + ag->attrs =NULL ;kfree (type->topology); } +static int skx_iio_set_mapping (struct intel_uncore_type *type) +{ +return pmu_iio_set_mapping (type, &skx_iio_mapping_group); +} + +static void skx_iio_cleanup_mapping (struct intel_uncore_type *type) +{ +pmu_iio_cleanup_mapping (type, &skx_iio_mapping_group); +} +static struct intel_uncore_type skx_uncore_iio = { .name ="iio" , .num_counters =4 ,
@@ -4501,6 +4507,11 @@ static int snr_iio_set_mapping(struct inreturn pmu_iio_set_mapping (type, &snr_iio_mapping_group); } +static void snr_iio_cleanup_mapping (struct intel_uncore_type *type) +{ +pmu_iio_cleanup_mapping (type, &snr_iio_mapping_group); +} +static struct intel_uncore_type snr_uncore_iio = { .name ="iio" , .num_counters =4 ,
@@ -4517,7 +4528,7 @@ static struct intel_uncore_type snr_unco .attr_update = snr_iio_attr_update, .get_topology = snr_iio_get_topology, .set_mapping = snr_iio_set_mapping, - .cleanup_mapping = skx_iio_cleanup_mapping, + .cleanup_mapping = snr_iio_cleanup_mapping, };static struct intel_uncore_type snr_uncore_irp = {
@@ -5092,6 +5103,11 @@ static int icx_iio_set_mapping(struct inreturn pmu_iio_set_mapping (type, &icx_iio_mapping_group); } +static void icx_iio_cleanup_mapping (struct intel_uncore_type *type) +{ +pmu_iio_cleanup_mapping (type, &icx_iio_mapping_group); +} +static struct intel_uncore_type icx_uncore_iio = { .name ="iio" , .num_counters =4 ,
@@ -5109,7 +5125,7 @@ static struct intel_uncore_type icx_unco .attr_update = icx_iio_attr_update, .get_topology = icx_iio_get_topology, .set_mapping = icx_iio_set_mapping, - .cleanup_mapping = skx_iio_cleanup_mapping, + .cleanup_mapping = icx_iio_cleanup_mapping, };static struct intel_uncore_type icx_uncore_irp = {
Greg Kroah-Hartman
maintainerPATCH 310/334 Revert "KVM: x86: mmu: Add guest physical address check in translate_gpa()"
In reply to Greg Kroah-HartmanFrom: Sean Christopherson <[email protected]>
commit e7177339d7b5f9594b316842122b5fda9513d5e2 upstream.
Revert a misguided illegal GPA check when "translating" a non-nested GPA. The check is woefully incomplete as it does not fill in @exception as expected by all callers, which leads to KVM attempting to inject a bogus exception, potentially exposing kernel stack information in the process.
WARNING: CPU: 0 PID: 8469 at arch/x86/kvm/x86.c:525 exception_type+0x98/0xb0 arch/x86/kvm/x86.c:525 CPU: 1 PID: 8469 Comm: syz-executor531 Not tainted 5.14.0-rc7-syzkaller #0 RIP: 0010:exception_type+0x98/0xb0 arch/x86/kvm/x86.c:525 Call Trace: x86_emulate_instruction+0xef6/0x1460 arch/x86/kvm/x86.c:7853 kvm_mmu_page_fault+0x2f0/0x1810 arch/x86/kvm/mmu/mmu.c:5199 handle_ept_misconfig+0xdf/0x3e0 arch/x86/kvm/vmx/vmx.c:5336 __vmx_handle_exit arch/x86/kvm/vmx/vmx.c:6021 [inline] vmx_handle_exit+0x336/0x1800 arch/x86/kvm/vmx/vmx.c:6038 vcpu_enter_guest+0x2a1c/0x4430 arch/x86/kvm/x86.c:9712 vcpu_run arch/x86/kvm/x86.c:9779 [inline] kvm_arch_vcpu_ioctl_run+0x47d/0x1b20 arch/x86/kvm/x86.c:10010 kvm_vcpu_ioctl+0x49e/0xe50 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3652
The bug has escaped notice because practically speaking the GPA check is useless. The GPA check in question only comes into play when KVM is walking guest page tables (or "translating" CR3), and KVM already handles illegal GPA checks by setting reserved bits in rsvd_bits_mask for each PxE, or in the case of CR3 for loading PTDPTRs, manually checks for an illegal CR3. This particular failure doesn't hit the existing reserved bits checks because syzbot sets guest.MAXPHYADDR=1, and IA32 architecture simply doesn't allow for such an absurd MAXPHYADDR, e.g. 32-bit paging doesn't define any reserved PA bits checks, which KVM emulates by only incorporating the reserved PA bits into the "high" bits, i.e. bits 63:32.
Simply remove the bogus check. There is zero meaningful value and no architectural justification for supporting guest.MAXPHYADDR < 32, and properly filling the exception would introduce non-trivial complexity.
This reverts commit ec7771ab471ba6a945350353617e2e3385d0e013.
Message-Id: <[email protected]>
--- arch/x86/kvm/mmu/mmu.c | 6 ------ 1 file changed, 6 deletions(-)
arch/x86/kvm/mmu/mmu.c +0 -6
@@ -323,12 +323,6 @@ static bool check_mmio_spte(struct kvm_vstatic gpa_t translate_gpa (struct kvm_vcpu *vcpu,gpa_t gpa,u32 access,struct x86_exception *exception) { -/* Check if guest physical address doesn't exceed guest maximum */ -if (kvm_vcpu_is_illegal_gpa (vcpu, gpa)) { - exception->error_code |=PFERR_RSVD_MASK ; -return UNMAPPED_GVA ; - } -return gpa; }
Greg Kroah-Hartman
maintainerPATCH 311/334 KVM: s390: index kvm->arch.idle_mask by vcpu_idx
In reply to Greg Kroah-HartmanFrom: Halil Pasic <[email protected]>
commit a3e03bc1368c1bc16e19b001fc96dc7430573cc8 upstream.
While in practice vcpu->vcpu_idx == vcpu->vcp_id is often true, it may not always be, and we must not rely on this. Reason is that KVM decides the vcpu_idx, userspace decides the vcpu_id, thus the two might not match.
Currently kvm->arch.idle_mask is indexed by vcpu_id, which implies that code like
for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
vcpu = kvm_get_vcpu(kvm, vcpu_id);
do_stuff(vcpu);
}is not legit. Reason is that kvm_get_vcpu expects an vcpu_idx, not an vcpu_id. The trouble is, we do actually use kvm->arch.idle_mask like this. To fix this problem we have two options. Either use kvm_get_vcpu_by_id(vcpu_id), which would loop to find the right vcpu_id, or switch to indexing via vcpu_idx. The latter is preferable for obvious reasons.
Let us make switch from indexing kvm->arch.idle_mask by vcpu_id to indexing it by vcpu_idx. To keep gisa_int.kicked_mask indexed by the same index as idle_mask lets make the same change for it as well.
--- arch/s390/include/asm/kvm_host.h | 1 + arch/s390/kvm/interrupt.c | 12 ++++++------ arch/s390/kvm/kvm-s390.c | 2 +- arch/s390/kvm/kvm-s390.h | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-)
arch/s390/include/asm/kvm_host.h +1 -0
@@ -957,6 +957,7 @@ struct kvm_arch{atomic64_t cmma_dirty_pages;/* subset of available cpu features enabled by user space */ DECLARE_BITMAP (cpu_feat,KVM_S390_VM_CPU_FEAT_NR_BITS ); +/* indexed by vcpu_idx */ DECLARE_BITMAP (idle_mask,KVM_MAX_VCPUS );struct kvm_s390_gisa_interrupt gisa_int;struct kvm_s390_pv pv;
arch/s390/kvm/interrupt.c +6 -6
@@ -419,13 +419,13 @@ static unsigned long deliverable_irqs(ststatic void __set_cpu_idle (struct kvm_vcpu *vcpu) {kvm_s390_set_cpuflags (vcpu,CPUSTAT_WAIT ); -set_bit (vcpu->vcpu_id, vcpu->kvm->arch.idle_mask); +set_bit (kvm_vcpu_get_idx (vcpu), vcpu->kvm->arch.idle_mask); }static void __unset_cpu_idle (struct kvm_vcpu *vcpu) {kvm_s390_clear_cpuflags (vcpu,CPUSTAT_WAIT ); -clear_bit (vcpu->vcpu_id, vcpu->kvm->arch.idle_mask); +clear_bit (kvm_vcpu_get_idx (vcpu), vcpu->kvm->arch.idle_mask); }static void __reset_intercept_indicators (struct kvm_vcpu *vcpu)
@@ -3050,18 +3050,18 @@ int kvm_s390_get_irq_state(struct kvm_vcstatic void __airqs_kick_single_vcpu (struct kvm *kvm,u8 deliverable_mask) { -int vcpu_id, online_vcpus =atomic_read (&kvm->online_vcpus); +int vcpu_idx, online_vcpus =atomic_read (&kvm->online_vcpus);struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;struct kvm_vcpu *vcpu; -for_each_set_bit (vcpu_id, kvm->arch.idle_mask, online_vcpus) { - vcpu =kvm_get_vcpu (kvm, vcpu_id); +for_each_set_bit (vcpu_idx, kvm->arch.idle_mask, online_vcpus) { + vcpu =kvm_get_vcpu (kvm, vcpu_idx);if (psw_ioint_disabled (vcpu))continue ; deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6 ] >>24 );if (deliverable_mask) {/* lately kicked but not yet running */ -if (test_and_set_bit (vcpu_id, gi->kicked_mask)) +if (test_and_set_bit (vcpu_idx, gi->kicked_mask))return ;kvm_s390_vcpu_wakeup (vcpu);return ;
arch/s390/kvm/kvm-s390.c +1 -1
@@ -4044,7 +4044,7 @@ static int vcpu_pre_run(struct kvm_vcpukvm_s390_patch_guest_per_regs (vcpu); } -clear_bit (vcpu->vcpu_id, vcpu->kvm->arch.gisa_int.kicked_mask); +clear_bit (kvm_vcpu_get_idx (vcpu), vcpu->kvm->arch.gisa_int.kicked_mask); vcpu->arch.sie_block->icptcode =0 ; cpuflags =atomic_read (&vcpu->arch.sie_block->cpuflags);
arch/s390/kvm/kvm-s390.h +1 -1
@@ -79,7 +79,7 @@ static inline int is_vcpu_stopped(structstatic inline int is_vcpu_idle (struct kvm_vcpu *vcpu) { -return test_bit (vcpu->vcpu_id, vcpu->kvm->arch.idle_mask); +return test_bit (kvm_vcpu_get_idx (vcpu), vcpu->kvm->arch.idle_mask); }static inline int kvm_is_ucontrol (struct kvm *kvm)
Greg Kroah-Hartman
maintainerPATCH 312/334 KVM: x86: Update vCPUs hv_clock before back to guest when tsc_offset is adjusted
In reply to Greg Kroah-HartmanFrom: Zelin Deng <[email protected]>
commit d9130a2dfdd4b21736c91b818f87dbc0ccd1e757 upstream.
When MSR_IA32_TSC_ADJUST is written by guest due to TSC ADJUST feature especially there's a big tsc warp (like a new vCPU is hot-added into VM which has been up for a long time), tsc_offset is added by a large value then go back to guest. This causes system time jump as tsc_timestamp is not adjusted in the meantime and pvclock monotonic character. To fix this, just notify kvm to update vCPU's guest time before back to guest.
Message-Id: <[email protected]>
--- arch/x86/kvm/x86.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/kvm/x86.c +4 -0
@@ -3316,6 +3316,10 @@ int kvm_set_msr_common(struct kvm_vcpu *if (!msr_info->host_initiated) {s64 adj = data - vcpu->arch.ia32_tsc_adjust_msr;adjust_tsc_offset_guest (vcpu, adj); +/* Before back to guest, tsc_timestamp must be adjusted +* as well, otherwise guest's percpu pvclock time could jump. +*/ +kvm_make_request (KVM_REQ_CLOCK_UPDATE , vcpu); } vcpu->arch.ia32_tsc_adjust_msr = data; }
Greg Kroah-Hartman
maintainerPATCH 313/334 KVM: x86: clamp host mapping level to max_level in kvm_mmu_max_mapping_level
In reply to Greg Kroah-HartmanFrom: Paolo Bonzini <[email protected]>
commit ec607a564f70519b340f7eb4cfc0f4a6b55285ac upstream.
This change started as a way to make kvm_mmu_hugepage_adjust a bit simpler, but it does fix two bugs as well.
One bug is in zapping collapsible PTEs. If a large page size is disallowed but not all of them, kvm_mmu_max_mapping_level will return the host mapping level and the small PTEs will be zapped up to that level. However, if e.g. 1GB are prohibited, we can still zap 4KB mapping and preserve the 2MB ones. This can happen for example when NX huge pages are in use.
The second would happen when userspace backs guest memory with a 1gb hugepage but only assign a subset of the page to the guest. 1gb pages would be disallowed by the memslot, but not 2mb. kvm_mmu_max_mapping_level() would fall through to the host_pfn_mapping_level() logic, see the 1gb hugepage, and map the whole thing into the guest.
--- arch/x86/kvm/mmu/mmu.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
arch/x86/kvm/mmu/mmu.c +5 -8
@@ -2846,6 +2846,7 @@ int kvm_mmu_max_mapping_level(struct kvm kvm_pfn_t pfn, int max_level) {struct kvm_lpage_info *linfo; +int host_level; max_level =min (max_level, max_huge_page_level);for ( ; max_level >PG_LEVEL_4K ; max_level--) {
@@ -2857,7 +2858,8 @@ int kvm_mmu_max_mapping_level(struct kvmif (max_level ==PG_LEVEL_4K )return PG_LEVEL_4K ; -return host_pfn_mapping_level (kvm, gfn, pfn, slot); + host_level =host_pfn_mapping_level (kvm, gfn, pfn, slot); +return min (host_level, max_level); } int kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu, gfn_t gfn,
@@ -2881,17 +2883,12 @@ int kvm_mmu_hugepage_adjust(struct kvm_vif (!slot)return PG_LEVEL_4K ; - level =kvm_mmu_max_mapping_level (vcpu->kvm, slot, gfn, pfn, max_level); -if (level ==PG_LEVEL_4K ) -return level; - - *req_level = level =min (level, max_level); -/* * Enforce the iTLB multihit workaround after capturing the requested * level, which will be used to do precise, accurate accounting. */ -if (huge_page_disallowed) + *req_level = level =kvm_mmu_max_mapping_level (vcpu->kvm, slot, gfn, pfn, max_level); +if (level ==PG_LEVEL_4K || huge_page_disallowed)return PG_LEVEL_4K ;/*
Greg Kroah-Hartman
maintainerPATCH 314/334 KVM: x86/mmu: Avoid collision with !PRESENT SPTEs in TDP MMU lpage stats
In reply to Greg Kroah-HartmanFrom: Sean Christopherson <[email protected]>
commit 088acd23526647844aec1c39db4ad02552c86c7b upstream.
Factor in whether or not the old/new SPTEs are shadow-present when adjusting the large page stats in the TDP MMU. A modified MMIO SPTE can toggle the page size bit, as bit 7 is used to store the MMIO generation, i.e. is_large_pte() can get a false positive when called on a MMIO SPTE. Ditto for nuking SPTEs with REMOVED_SPTE, which sets bit 7 in its magic value.
Opportunistically move the logic below the check to verify at least one of the old/new SPTEs is shadow present.
Use is/was_leaf even though is/was_present would suffice. The code generation is roughly equivalent since all flags need to be computed prior to the code in question, and using the *_leaf flags will minimize the diff in a future enhancement to account all pages, i.e. will change the check to "is_leaf != was_leaf".
Message-Id: <[email protected]>
--- arch/x86/kvm/mmu/tdp_mmu.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
arch/x86/kvm/mmu/tdp_mmu.c +13 -7
@@ -412,6 +412,7 @@ static void __handle_changed_spte(structbool was_leaf = was_present &&is_last_spte (old_spte, level);bool is_leaf = is_present &&is_last_spte (new_spte, level);bool pfn_changed =spte_to_pfn (old_spte) !=spte_to_pfn (new_spte); +bool was_large, is_large;WARN_ON (level >PT64_ROOT_MAX_LEVEL );WARN_ON (level <PG_LEVEL_4K );
@@ -445,13 +446,6 @@ static void __handle_changed_spte(structtrace_kvm_tdp_mmu_spte_changed (as_id, gfn, level, old_spte, new_spte); -if (is_large_pte (old_spte) !=is_large_pte (new_spte)) { -if (is_large_pte (old_spte)) -atomic64_sub (1 , (atomic64_t *)&kvm->stat.lpages); -else -atomic64_add (1 , (atomic64_t *)&kvm->stat.lpages); - } -/* * The only times a SPTE should be changed from a non-present to * non-present state is when an MMIO entry is installed/modified/
@@ -477,6 +471,18 @@ static void __handle_changed_spte(structreturn ; } +/* +* Update large page stats if a large page is being zapped, created, or +* is replacing an existing shadow page. +*/ + was_large = was_leaf &&is_large_pte (old_spte); + is_large = is_leaf &&is_large_pte (new_spte); +if (was_large != is_large) { +if (was_large) +atomic64_sub (1 , (atomic64_t *)&kvm->stat.lpages); +else +atomic64_add (1 , (atomic64_t *)&kvm->stat.lpages); + }if (was_leaf &&is_dirty_spte (old_spte) && (!is_present || !is_dirty_spte (new_spte) || pfn_changed))
Greg Kroah-Hartman
maintainerPATCH 315/334 KVM: VMX: avoid running vmx_handle_exit_irqoff in case of emulation
In reply to Greg Kroah-HartmanFrom: Maxim Levitsky <[email protected]>
commit 81b4b56d4f8130bbb99cf4e2b48082e5b4cfccb9 upstream.
If we are emulating an invalid guest state, we don't have a correct exit reason, and thus we shouldn't do anything in this function.
Message-Id: <[email protected]>
--- arch/x86/kvm/vmx/vmx.c | 3 +++ 1 file changed, 3 insertions(+)
arch/x86/kvm/vmx/vmx.c +3 -0
@@ -6368,6 +6368,9 @@ static void vmx_handle_exit_irqoff(struc {struct vcpu_vmx *vmx =to_vmx (vcpu); +if (vmx->emulation_required) +return ; +if (vmx->exit_reason.basic ==EXIT_REASON_EXTERNAL_INTERRUPT )handle_external_interrupt_irqoff (vcpu);else if (vmx->exit_reason.basic ==EXIT_REASON_EXCEPTION_NMI )
Greg Kroah-Hartman
maintainerPATCH 316/334 KVM: nVMX: Unconditionally clear nested.pi_pending on nested VM-Enter
In reply to Greg Kroah-HartmanFrom: Sean Christopherson <[email protected]>
commit f7782bb8d818d8f47c26b22079db10599922787a upstream.
Clear nested.pi_pending on nested VM-Enter even if L2 will run without posted interrupts enabled. If nested.pi_pending is left set from a previous L2, vmx_complete_nested_posted_interrupt() will pick up the stale flag and exit to userspace with an "internal emulation error" due the new L2 not having a valid nested.pi_desc.
Arguably, vmx_complete_nested_posted_interrupt() should first check for posted interrupts being enabled, but it's also completely reasonable that KVM wouldn't screw up a fundamental flag. Not to mention that the mere existence of nested.pi_pending is a long-standing bug as KVM shouldn't move the posted interrupt out of the IRR until it's actually processed, e.g. KVM effectively drops an interrupt when it performs a nested VM-Exit with a "pending" posted interrupt. Fixing the mess is a future problem.
Prior to vmx_complete_nested_posted_interrupt() interpreting a null PI descriptor as an error, this was a benign bug as the null PI descriptor effectively served as a check on PI not being enabled. Even then, the new flow did not become problematic until KVM started checking the result of kvm_check_nested_events().
Message-Id: <[email protected]>
--- arch/x86/kvm/vmx/nested.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
arch/x86/kvm/vmx/nested.c +3 -4
@@ -2223,12 +2223,11 @@ static void prepare_vmcs02_early(struct ~PIN_BASED_VMX_PREEMPTION_TIMER);/* Posted interrupts setting is only taken from vmcs12. */ -if (nested_cpu_has_posted_intr (vmcs12)) { + vmx->nested.pi_pending = false; +if (nested_cpu_has_posted_intr (vmcs12)) vmx->nested.posted_intr_nv = vmcs12->posted_intr_nv; - vmx->nested.pi_pending = false; - }else { +else exec_control &= ~PIN_BASED_POSTED_INTR ; - }pin_controls_set (vmx, exec_control);/*
Greg Kroah-Hartman
maintainerPATCH 317/334 KVM: arm64: Unregister HYP sections from kmemleak in protected mode
In reply to Greg Kroah-HartmanFrom: Marc Zyngier <[email protected]>
commit 47e6223c841e029bfc23c3ce594dac5525cebaf8 upstream.
Booting a KVM host in protected mode with kmemleak quickly results in a pretty bad crash, as kmemleak doesn't know that the HYP sections have been taken away. This is specially true for the BSS section, which is part of the kernel BSS section and registered at boot time by kmemleak itself.
Unregister the HYP part of the BSS before making that section HYP-private. The rest of the HYP-specific data is obtained via the page allocator or lives in other sections, none of which is subjected to kmemleak.
--- arch/arm64/kvm/arm.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/kvm/arm.c +7 -0
@@ -15,6 +15,7 @@#include <linux/fs.h> #include <linux/mman.h> #include <linux/sched.h> +#include <linux/kmemleak.h> #include <linux/kvm.h> #include <linux/kvm_irqfd.h> #include <linux/irqbypass.h>
@@ -1986,6 +1987,12 @@ static int finalize_hyp_mode(void)if (ret)return ret; +/* +* Exclude HYP BSS from kmemleak so that it doesn't get peeked +* at, which would end badly once the section is inaccessible. +* None of other sections should ever be introspected. +*/ +kmemleak_free_part (__hyp_bss_start, __hyp_bss_end - __hyp_bss_start); ret =pkvm_mark_hyp_section (__hyp_bss);if (ret)return ret;
Greg Kroah-Hartman
maintainerPATCH 318/334 KVM: arm64: vgic: Resample HW pending state on deactivation
In reply to Greg Kroah-HartmanFrom: Marc Zyngier <[email protected]>
commit 3134cc8beb69d0db9de651081707c4651c011621 upstream.
When a mapped level interrupt (a timer, for example) is deactivated by the guest, the corresponding host interrupt is equally deactivated. However, the fate of the pending state still needs to be dealt with in SW.
This is specially true when the interrupt was in the active+pending state in the virtual distributor at the point where the guest was entered. On exit, the pending state is potentially stale (the guest may have put the interrupt in a non-pending state).
If we don't do anything, the interrupt will be spuriously injected in the guest. Although this shouldn't have any ill effect (spurious interrupts are always possible), we can improve the emulation by detecting the deactivation-while-pending case and resample the interrupt.
While we're at it, move the logic into a common helper that can be shared between the two GIC implementations.
--- arch/arm64/kvm/vgic/vgic-v2.c | 36 +++++------------------------------- arch/arm64/kvm/vgic/vgic-v3.c | 36 +++++------------------------------- arch/arm64/kvm/vgic/vgic.c | 38 ++++++++++++++++++++++++++++++++++++++ arch/arm64/kvm/vgic/vgic.h | 2 ++ 4 files changed, 50 insertions(+), 62 deletions(-)
arch/arm64/kvm/vgic/vgic-v2.c +5 -31
@@ -60,6 +60,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcu32 val = cpuif->vgic_lr[lr];u32 cpuid, intid = val &GICH_LR_VIRTUALID ;struct vgic_irq *irq; +bool deactivated;/* Extract the source vCPU id from the LR */ cpuid = val &GICH_LR_PHYSID_CPUID ;
@@ -75,7 +76,8 @@ void vgic_v2_fold_lr_state(struct kvm_vcraw_spin_lock (&irq->irq_lock); -/* Always preserve the active bit */ +/* Always preserve the active bit, note deactivation */ + deactivated = irq->active && !(val &GICH_LR_ACTIVE_BIT ); irq->active = !!(val &GICH_LR_ACTIVE_BIT );if (irq->active &&vgic_irq_is_sgi (intid))
@@ -96,36 +98,8 @@ void vgic_v2_fold_lr_state(struct kvm_vcif (irq->config ==VGIC_CONFIG_LEVEL && !(val &GICH_LR_STATE )) irq->pending_latch = false; -/* -* Level-triggered mapped IRQs are special because we only -* observe rising edges as input to the VGIC. -* -* If the guest never acked the interrupt we have to sample -* the physical line and set the line level, because the -* device state could have changed or we simply need to -* process the still pending interrupt later. -* -* If this causes us to lower the level, we have to also clear -* the physical active state, since we will otherwise never be -* told when the interrupt becomes asserted again. -* -* Another case is when the interrupt requires a helping hand -* on deactivation (no HW deactivation, for example). -*/ -if (vgic_irq_is_mapped_level (irq)) { -bool resample = false; - -if (val &GICH_LR_PENDING_BIT ) { - irq->line_level =vgic_get_phys_line_level (irq); - resample = !irq->line_level; - }else if (vgic_irq_needs_resampling (irq) && - !(irq->active || irq->pending_latch)) { - resample = true; - } - -if (resample) -vgic_irq_set_phys_active (irq, false); - } +/* Handle resampling for mapped interrupts if required */ +vgic_irq_handle_resampling (irq, deactivated, val &GICH_LR_PENDING_BIT );raw_spin_unlock (&irq->irq_lock);vgic_put_irq (vcpu->kvm, irq);
arch/arm64/kvm/vgic/vgic-v3.c +5 -31
@@ -46,6 +46,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcu32 intid, cpuid;struct vgic_irq *irq;bool is_v2_sgi = false; +bool deactivated; cpuid = val &GICH_LR_PHYSID_CPUID ; cpuid >>=GICH_LR_PHYSID_CPUID_SHIFT ;
@@ -68,7 +69,8 @@ void vgic_v3_fold_lr_state(struct kvm_vcraw_spin_lock (&irq->irq_lock); -/* Always preserve the active bit */ +/* Always preserve the active bit, note deactivation */ + deactivated = irq->active && !(val &ICH_LR_ACTIVE_BIT ); irq->active = !!(val &ICH_LR_ACTIVE_BIT );if (irq->active && is_v2_sgi)
@@ -89,36 +91,8 @@ void vgic_v3_fold_lr_state(struct kvm_vcif (irq->config ==VGIC_CONFIG_LEVEL && !(val &ICH_LR_STATE )) irq->pending_latch = false; -/* -* Level-triggered mapped IRQs are special because we only -* observe rising edges as input to the VGIC. -* -* If the guest never acked the interrupt we have to sample -* the physical line and set the line level, because the -* device state could have changed or we simply need to -* process the still pending interrupt later. -* -* If this causes us to lower the level, we have to also clear -* the physical active state, since we will otherwise never be -* told when the interrupt becomes asserted again. -* -* Another case is when the interrupt requires a helping hand -* on deactivation (no HW deactivation, for example). -*/ -if (vgic_irq_is_mapped_level (irq)) { -bool resample = false; - -if (val &ICH_LR_PENDING_BIT ) { - irq->line_level =vgic_get_phys_line_level (irq); - resample = !irq->line_level; - }else if (vgic_irq_needs_resampling (irq) && - !(irq->active || irq->pending_latch)) { - resample = true; - } - -if (resample) -vgic_irq_set_phys_active (irq, false); - } +/* Handle resampling for mapped interrupts if required */ +vgic_irq_handle_resampling (irq, deactivated, val &ICH_LR_PENDING_BIT );raw_spin_unlock (&irq->irq_lock);vgic_put_irq (vcpu->kvm, irq);
arch/arm64/kvm/vgic/vgic.c +38 -0
@@ -1022,3 +1022,41 @@ bool kvm_vgic_map_is_active(struct kvm_vreturn map_is_active; } + +/* +* Level-triggered mapped IRQs are special because we only observe rising +* edges as input to the VGIC. +* +* If the guest never acked the interrupt we have to sample the physical +* line and set the line level, because the device state could have changed +* or we simply need to process the still pending interrupt later. +* +* We could also have entered the guest with the interrupt active+pending. +* On the next exit, we need to re-evaluate the pending state, as it could +* otherwise result in a spurious interrupt by injecting a now potentially +* stale pending state. +* +* If this causes us to lower the level, we have to also clear the physical +* active state, since we will otherwise never be told when the interrupt +* becomes asserted again. +* +* Another case is when the interrupt requires a helping hand on +* deactivation (no HW deactivation, for example). +*/ +void vgic_irq_handle_resampling (struct vgic_irq *irq, +bool lr_deactivated,bool lr_pending) +{ +if (vgic_irq_is_mapped_level (irq)) { +bool resample = false; + +if (unlikely (vgic_irq_needs_resampling (irq))) { + resample = !(irq->active || irq->pending_latch); + }else if (lr_pending || (lr_deactivated && irq->line_level)) { + irq->line_level =vgic_get_phys_line_level (irq); + resample = !irq->line_level; + } + +if (resample) +vgic_irq_set_phys_active (irq, false); + } +}
arch/arm64/kvm/vgic/vgic.h +2 -0
@@ -169,6 +169,8 @@ void vgic_irq_set_phys_active(struct vgibool vgic_queue_irq_unlock (struct kvm *kvm,struct vgic_irq *irq,unsigned long flags);void vgic_kick_vcpus (struct kvm *kvm); +void vgic_irq_handle_resampling (struct vgic_irq *irq, +bool lr_deactivated,bool lr_pending);int vgic_check_ioaddr (struct kvm *kvm,phys_addr_t *ioaddr,phys_addr_t addr,phys_addr_t alignment);
Greg Kroah-Hartman
maintainerPATCH 319/334 ARM: dts: at91: add pinctrl-{names, 0} for all gpios
In reply to Greg Kroah-HartmanFrom: Claudiu Beznea <[email protected]>
commit bf781869e5cf3e4ec1a47dad69b6f0df97629cbd upstream.
Add pinctrl-names and pinctrl-0 properties on controllers that claims to use pins to avoid failures due to
commit 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges") and also to avoid using pins that may be claimed my other IPs.
--- arch/arm/boot/dts/at91-sam9x60ek.dts | 16 ++++++++++++++- arch/arm/boot/dts/at91-sama5d3_xplained.dts | 29 ++++++++++++++++++++++++++++ arch/arm/boot/dts/at91-sama5d4_xplained.dts | 19 ++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-)
arch/arm/boot/dts/at91-sam9x60ek.dts +15 -1
@@ -92,6 +92,8 @@ leds { compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_gpio_leds>; status = "okay"; /* Conflict with pwm0. */ red {
@@ -537,6 +539,10 @@ AT91_PIOA 19 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI) /* PA19 DAT2 periph A with pullup */ AT91_PIOA 20 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI)>; /* PA20 DAT3 periph A with pullup */ }; + pinctrl_sdmmc0_cd: sdmmc0_cd { + atmel,pins = + <AT91_PIOA 23 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; }; sdmmc1 {
@@ -569,6 +575,14 @@ AT91_PIOD 16 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; }; }; + + leds { + pinctrl_gpio_leds: gpio_leds { + atmel,pins = <AT91_PIOB 11 AT91_PERIPH_GPIO AT91_PINCTRL_NONE + AT91_PIOB 12 AT91_PERIPH_GPIO AT91_PINCTRL_NONE + AT91_PIOB 13 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + }; }; /* pinctrl */ &pwm0 {
@@ -580,7 +594,7 @@ &sdmmc0 { bus-width = <4>; pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_sdmmc0_default>; + pinctrl-0 = <&pinctrl_sdmmc0_default &pinctrl_sdmmc0_cd>; status = "okay"; cd-gpios = <&pioA 23 GPIO_ACTIVE_LOW>; disable-wp;
arch/arm/boot/dts/at91-sama5d3_xplained.dts +29 -0
@@ -57,6 +57,8 @@ }; spi0: spi@f0004000 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi0_cs>; cs-gpios = <&pioD 13 0>, <0>, <0>, <&pioD 16 0>; status = "okay"; };
@@ -169,6 +171,8 @@ }; spi1: spi@f8008000 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi1_cs>; cs-gpios = <&pioC 25 0>; status = "okay"; };
@@ -248,6 +252,26 @@ <AT91_PIOE 3 AT91_PERIPH_GPIO AT91_PINCTRL_NONE AT91_PIOE 4 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; }; + + pinctrl_gpio_leds: gpio_leds_default { + atmel,pins = + <AT91_PIOE 23 AT91_PERIPH_GPIO AT91_PINCTRL_NONE + AT91_PIOE 24 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + + pinctrl_spi0_cs: spi0_cs_default { + atmel,pins = + <AT91_PIOD 13 AT91_PERIPH_GPIO AT91_PINCTRL_NONE + AT91_PIOD 16 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + + pinctrl_spi1_cs: spi1_cs_default { + atmel,pins = <AT91_PIOC 25 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + + pinctrl_vcc_mmc0_reg_gpio: vcc_mmc0_reg_gpio_default { + atmel,pins = <AT91_PIOE 2 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; }; }; };
@@ -339,6 +363,8 @@ vcc_mmc0_reg: fixedregulator_mmc0 { compatible = "regulator-fixed"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_vcc_mmc0_reg_gpio>; gpio = <&pioE 2 GPIO_ACTIVE_LOW>; regulator-name = "mmc0-card-supply"; regulator-min-microvolt = <3300000>;
@@ -362,6 +388,9 @@ leds { compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_gpio_leds>; + status = "okay"; d2 { label = "d2";
arch/arm/boot/dts/at91-sama5d4_xplained.dts +19 -0
@@ -90,6 +90,8 @@ }; spi1: spi@fc018000 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi0_cs>; cs-gpios = <&pioB 21 0>; status = "okay"; };
@@ -147,6 +149,19 @@ atmel,pins = <AT91_PIOE 1 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>; }; + pinctrl_spi0_cs: spi0_cs_default { + atmel,pins = + <AT91_PIOB 21 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + pinctrl_gpio_leds: gpio_leds_default { + atmel,pins = + <AT91_PIOD 30 AT91_PERIPH_GPIO AT91_PINCTRL_NONE + AT91_PIOE 15 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; + pinctrl_vcc_mmc1_reg: vcc_mmc1_reg { + atmel,pins = + <AT91_PIOE 4 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; + }; }; }; };
@@ -252,6 +267,8 @@ leds { compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_gpio_leds>; status = "okay"; d8 {
@@ -278,6 +295,8 @@ vcc_mmc1_reg: fixedregulator_mmc1 { compatible = "regulator-fixed"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_vcc_mmc1_reg>; gpio = <&pioE 4 GPIO_ACTIVE_LOW>; regulator-name = "VDD MCI1"; regulator-min-microvolt = <3300000>;
Greg Kroah-Hartman
maintainerPATCH 320/334 io-wq: check max_worker limits if a worker transitions bound state
In reply to Greg Kroah-HartmanFrom: Jens Axboe <[email protected]>
commit ecc53c48c13d995e6fe5559e30ffee48d92784fd upstream.
For the two places where new workers are created, we diligently check if we are allowed to create a new worker. If we're currently at the limit of how many workers of a given type we can have, then we don't create any new ones.
If you have a mixed workload with various types of bound and unbounded work, then it can happen that a worker finishes one type of work and is then transitioned to the other type. For this case, we don't check if we are actually allowed to do so. This can cause io-wq to temporarily exceed the allowed number of workers for a given type.
When retrieving work, check that the types match. If they don't, check if we are allowed to transition to the other type. If not, then don't handle the new work.
--- fs/io-wq.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
fs/io-wq.c +30 -3
@@ -423,7 +423,28 @@ static void io_wait_on_hash(struct io_wqspin_unlock (&wq->hash->wait.lock); } -static struct io_wq_work *io_get_next_work (struct io_wqe *wqe) +/* +* We can always run the work if the worker is currently the same type as +* the work (eg both are bound, or both are unbound). If they are not the +* same, only allow it if incrementing the worker count would be allowed. +*/ +static bool io_worker_can_run_work (struct io_worker *worker, +struct io_wq_work *work) +{ +struct io_wqe_acct *acct; + +if (!(worker->flags &IO_WORKER_F_BOUND ) != + !(work->flags &IO_WQ_WORK_UNBOUND )) +return true; + +/* not the same type, check if we'd go over the limit */ + acct =io_work_get_acct (worker->wqe, work); +return acct->nr_workers < acct->max_workers; +} + +static struct io_wq_work *io_get_next_work (struct io_wqe *wqe, +struct io_worker *worker, +bool *stalled) __must_hold(wqe->lock) {struct io_wq_work_node *node, *prev;
@@ -435,6 +456,9 @@ static struct io_wq_work *io_get_next_wo work = container_of(node, struct io_wq_work, list); +if (!io_worker_can_run_work (worker, work)) +break ; +/* not hashed, can run anytime */ if (!io_wq_is_hashed (work)) {wq_list_del (&wqe->work_list, node, prev);
@@ -461,6 +485,7 @@ static struct io_wq_work *io_get_next_woraw_spin_unlock (&wqe->lock);io_wait_on_hash (wqe, stall_hash);raw_spin_lock (&wqe->lock); + *stalled = true; }return NULL ;
@@ -500,6 +525,7 @@ static void io_worker_handle_work(struct do { struct io_wq_work *work; + bool stalled; get_next: /* * If we got some work, mark us as busy. If we didn't, but
@@ -508,10 +534,11 @@ get_next:* can't make progress, any work completion or insertion will * clear the stalled flag. */ - work =io_get_next_work (wqe); + stalled = false; + work =io_get_next_work (wqe, worker, &stalled);if (work)__io_worker_busy (wqe, worker, work); -else if (!wq_list_empty (&wqe->work_list)) +else if (stalled) wqe->flags |=IO_WQE_FLAG_STALLED ;raw_spin_unlock_irq (&wqe->lock);
Greg Kroah-Hartman
maintainerPATCH 321/334 md/raid10: Remove unnecessary rcu_dereference in raid10_handle_discard
In reply to Greg Kroah-HartmanFrom: Xiao Ni <[email protected]>
commit 46d4703b1db4c86ab5acb2331b10df999f005e8e upstream.
We are seeing the following warning in raid10_handle_discard.
[ 695.110751] ============================= [ 695.131439] WARNING: suspicious RCU usage [ 695.151389] 4.18.0-319.el8.x86_64+debug #1 Not tainted [ 695.174413] ----------------------------- [ 695.192603] drivers/md/raid10.c:1776 suspicious rcu_dereference_check() usage! [ 695.225107] other info that might help us debug this: [ 695.260940] rcu_scheduler_active = 2, debug_locks = 1 [ 695.290157] no locks held by mkfs.xfs/10186.
In the first loop of function raid10_handle_discard. It already determines which disk need to handle discard request and add the rdev reference count rdev->nr_pending. So the conf->mirrors will not change until all bios come back from underlayer disks. It doesn't need to use rcu_dereference to get rdev.
--- drivers/md/raid10.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/md/raid10.c +10 -4
@@ -1712,6 +1712,11 @@ retry_discard: }else r10_bio->master_bio = (struct bio *)first_r10bio; +/* +* first select target devices under rcu_lock and +* inc refcount on their rdev. Record them by setting +* bios[x] to bio +*/ rcu_read_lock ();for (disk =0 ; disk < geo->raid_disks; disk++) {struct md_rdev *rdev =rcu_dereference (conf->mirrors[disk].rdev);
@@ -1743,9 +1748,6 @@ retry_discard:for (disk =0 ; disk < geo->raid_disks; disk++) {sector_t dev_start, dev_end;struct bio *mbio, *rbio =NULL ; -struct md_rdev *rdev =rcu_dereference (conf->mirrors[disk].rdev); -struct md_rdev *rrdev =rcu_dereference ( - conf->mirrors[disk].replacement);/* * Now start to calculate the start and end address for each disk.
@@ -1775,9 +1777,12 @@ retry_discard:/* * It only handles discard bio which size is >= stripe size, so -* dev_end > dev_start all the time +* dev_end > dev_start all the time. +* It doesn't need to use rcu lock to get rdev here. We already +* add rdev->nr_pending in the first loop. */ if (r10_bio->devs[disk].bio) { +struct md_rdev *rdev = conf->mirrors[disk].rdev; mbio =bio_clone_fast (bio,GFP_NOIO , &mddev->bio_set); mbio->bi_end_io = raid10_end_discard_request; mbio->bi_private = r10_bio;
@@ -1790,6 +1795,7 @@ retry_discard:bio_endio (mbio); }if (r10_bio->devs[disk].repl_bio) { +struct md_rdev *rrdev = conf->mirrors[disk].replacement; rbio =bio_clone_fast (bio,GFP_NOIO , &mddev->bio_set); rbio->bi_end_io = raid10_end_discard_request; rbio->bi_private = r10_bio;
Greg Kroah-Hartman
maintainerPATCH 322/334 char: tpm: Kconfig: remove bad i2c cr50 select
In reply to Greg Kroah-HartmanFrom: Adrian Ratiu <[email protected]>
commit 847fdae1579f4ee930b01f24a7847b8043bf468c upstream.
This fixes a minor bug which went unnoticed during the initial driver upstreaming review: TCG_CR50 does not exist in mainline kernels, so remove it.
--- drivers/char/tpm/Kconfig | 1 - 1 file changed, 1 deletion(-)
drivers/char/tpm/Kconfig +0 -1
@@ -89,7 +89,6 @@ config TCG_TIS_SYNQUACER config TCG_TIS_I2C_CR50 tristate "TPM Interface Specification 2.0 Interface (I2C - CR50)" depends on I2C - select TCG_CR50 help This is a driver for the Google cr50 I2C TPM interface which is a custom microcontroller and requires a custom i2c protocol interface
Greg Kroah-Hartman
maintainerPATCH 323/334 fuse: truncate pagecache on atomic_o_trunc
In reply to Greg Kroah-HartmanFrom: Miklos Szeredi <[email protected]>
commit 76224355db7570cbe6b6f75c8929a1558828dd55 upstream.
fuse_finish_open() will be called with FUSE_NOWRITE in case of atomic O_TRUNC. This can deadlock with fuse_wait_on_page_writeback() in fuse_launder_page() triggered by invalidate_inode_pages2().
Fix by replacing invalidate_inode_pages2() in fuse_finish_open() with a truncate_pagecache() call. This makes sense regardless of FOPEN_KEEP_CACHE or fc->writeback cache, so do it unconditionally.
--- fs/fuse/file.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
fs/fuse/file.c +5 -2
@@ -198,12 +198,11 @@ void fuse_finish_open(struct inode *inodstruct fuse_file *ff = file->private_data;struct fuse_conn *fc =get_fuse_conn (inode); -if (!(ff->open_flags &FOPEN_KEEP_CACHE )) -invalidate_inode_pages2 (inode->i_mapping);if (ff->open_flags &FOPEN_STREAM )stream_open (inode, file);else if (ff->open_flags &FOPEN_NONSEEKABLE )nonseekable_open (inode, file); +if (fc->atomic_o_trunc && (file->f_flags &O_TRUNC )) {struct fuse_inode *fi =get_fuse_inode (inode);
@@ -211,10 +210,14 @@ void fuse_finish_open(struct inode *inod fi->attr_version =atomic64_inc_return (&fc->attr_version);i_size_write (inode,0 );spin_unlock (&fi->lock); +truncate_pagecache (inode,0 );fuse_invalidate_attr (inode);if (fc->writeback_cache)file_update_time (file); + }else if (!(ff->open_flags &FOPEN_KEEP_CACHE )) { +invalidate_inode_pages2 (inode->i_mapping); } +if ((file->f_mode &FMODE_WRITE ) && fc->writeback_cache)fuse_link_write_file (file); }
Greg Kroah-Hartman
maintainerPATCH 325/334 fuse: wait for writepages in syncfs
In reply to Greg Kroah-HartmanFrom: Miklos Szeredi <[email protected]>
commit 660585b56e63ca034ad506ea53c807c5cdca3196 upstream.
In case of fuse the MM subsystem doesn't guarantee that page writeback completes by the time ->sync_fs() is called. This is because fuse completes page writeback immediately to prevent DoS of memory reclaim by the userspace file server.
This means that fuse itself must ensure that writes are synced before sending the SYNCFS request to the server.
Introduce sync buckets, that hold a counter for the number of outstanding write requests. On syncfs replace the current bucket with a new one and wait until the old bucket's counter goes down to zero.
It is possible to have multiple syncfs calls in parallel, in which case there could be more than one waited-on buckets. Descendant buckets must not complete until the parent completes. Add a count to the child (new) bucket until the (parent) old bucket completes.
Use RCU protection to dereference the current bucket and to wake up an emptied bucket. Use fc->lock to protect against parallel assignments to the current bucket.
This leaves just the counter to be a possible scalability issue. The fc->num_waiting counter has a similar issue, so both should be addressed at the same time.
--- fs/fuse/file.c | 21 +++++++++++++++++++ fs/fuse/fuse_i.h | 19 +++++++++++++++++ fs/fuse/inode.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+)
fs/fuse/file.c +21 -0
@@ -392,6 +392,7 @@ struct fuse_writepage_args {struct list_head queue_entry;struct fuse_writepage_args *next;struct inode *inode; +struct fuse_sync_bucket *bucket; }; static struct fuse_writepage_args *fuse_find_writeback(struct fuse_inode *fi,
@@ -1611,6 +1612,9 @@ static void fuse_writepage_free(struct fstruct fuse_args_pages *ap = &wpa->ia.ap;int i; +if (wpa->bucket) +fuse_sync_bucket_dec (wpa->bucket); +for (i =0 ; i < ap->num_pages; i++)__free_page (ap->pages[i]);
@@ -1874,6 +1878,20 @@ static struct fuse_writepage_args *fuse_ } +static void fuse_writepage_add_to_bucket (struct fuse_conn *fc, +struct fuse_writepage_args *wpa) +{ +if (!fc->sync_fs) +return ; + +rcu_read_lock (); +/* Prevent resurrection of dead bucket in unlikely race with syncfs */ +do { + wpa->bucket =rcu_dereference (fc->curr_bucket); + }while (unlikely (!atomic_inc_not_zero (&wpa->bucket->count))); +rcu_read_unlock (); +} +static int fuse_writepage_locked (struct page *page) {struct address_space *mapping = page->mapping;
@@ -1901,6 +1919,7 @@ static int fuse_writepage_locked(structif (!wpa->ia.ff) goto err_nofile; +fuse_writepage_add_to_bucket (fc, wpa);fuse_write_args_fill (&wpa->ia, wpa->ia.ff,page_offset (page),0 );copy_highpage (tmp_page, page);
@@ -2151,6 +2170,8 @@ static int fuse_writepages_fill(struct p__free_page (tmp_page); goto out_unlock; } +fuse_writepage_add_to_bucket (fc, wpa); + data->max_pages =1 ; ap = &wpa->ia.ap;
fs/fuse/fuse_i.h +19 -0
@@ -515,6 +515,13 @@ struct fuse_fs_context {void **fudptr; }; +struct fuse_sync_bucket { +/* count is a possible scalability bottleneck */ +atomic_t count; +wait_queue_head_t waitq; +struct rcu_head rcu; +}; +/** * A Fuse connection. *
@@ -807,6 +814,9 @@ struct fuse_conn {/** List of filesystems using this connection */ struct list_head mounts; + +/* New writepages go into this bucket */ +struct fuse_sync_bucket __rcu *curr_bucket; };/*
@@ -910,6 +920,15 @@ static inline void fuse_page_descs_lengt descs[i].length =PAGE_SIZE - descs[i].offset; } +static inline void fuse_sync_bucket_dec (struct fuse_sync_bucket *bucket) +{ +/* Need RCU protection to prevent use after free after the decrement */ +rcu_read_lock (); +if (atomic_dec_and_test (&bucket->count)) +wake_up (&bucket->waitq); +rcu_read_unlock (); +} +/** Device operations */ extern const struct file_operations fuse_dev_operations;
fs/fuse/inode.c +60 -0
90 lines not on this page · raw message
Greg Kroah-Hartman
maintainerPATCH 326/334 IMA: remove -Wmissing-prototypes warning
In reply to Greg Kroah-HartmanFrom: Austin Kim <[email protected]>
commit a32ad90426a9c8eb3915eed26e08ce133bd9e0da upstream.
With W=1 build, the compiler throws warning message as below:
security/integrity/ima/ima_mok.c:24:12: warning:
no previous prototype for ‘ima_mok_init’ [-Wmissing-prototypes]
__init int ima_mok_init(void)
Silence the warning by adding static keyword to ima_mok_init().
--- security/integrity/ima/ima_mok.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
security/integrity/ima/ima_mok.c +1 -1
@@ -21,7 +21,7 @@ struct key *ima_blacklist_keyring;/* * Allocate the IMA blacklist keyring */ -__init int ima_mok_init (void ) +static __init int ima_mok_init (void ) {struct key_restriction *restriction;
Greg Kroah-Hartman
maintainerPATCH 327/334 IMA: remove the dependency on CRYPTO_MD5
In reply to Greg Kroah-HartmanFrom: THOBY Simon <[email protected]>
commit 8510505d55e194d3f6c9644c9f9d12c4f6b0395a upstream.
MD5 is a weak digest algorithm that shouldn't be used for cryptographic operation. It hinders the efficiency of a patch set that aims to limit the digests allowed for the extended file attribute namely security.ima. MD5 is no longer a requirement for IMA, nor should it be used there.
The sole place where we still use the MD5 algorithm inside IMA is setting the ima_hash algorithm to MD5, if the user supplies 'ima_hash=md5' parameter on the command line. With commit ab60368ab6a4 ("ima: Fallback to the builtin hash algorithm"), setting "ima_hash=md5" fails gracefully when CRYPTO_MD5 is not set:
ima: Can not allocate md5 (reason: -2)
ima: Allocating md5 failed, going to use default hash algorithm sha256
Remove the CRYPTO_MD5 dependency for IMA.
[[email protected]: include commit number in patch description for stable.]
--- security/integrity/ima/Kconfig | 1 - 1 file changed, 1 deletion(-)
security/integrity/ima/Kconfig +0 -1
@@ -6,7 +6,6 @@ config IMA select SECURITYFS select CRYPTO select CRYPTO_HMAC - select CRYPTO_MD5 select CRYPTO_SHA1 select CRYPTO_HASH_INFO select TCG_TPM if HAS_IOMEM && !UML
Greg Kroah-Hartman
maintainerPATCH 328/334 fbmem: dont allow too huge resolutions
In reply to Greg Kroah-HartmanFrom: Tetsuo Handa <[email protected]>
commit 8c28051cdcbe9dfcec6bd0a4709d67a09df6edae upstream.
syzbot is reporting page fault at vga16fb_fillrect() [1], for vga16fb_check_var() is failing to detect multiplication overflow.
if (vxres * vyres > maxmem) {
vyres = maxmem / vxres;
if (vyres < yres)
return -ENOMEM;
}Since no module would accept too huge resolutions where multiplication overflow happens, let's reject in the common path.
--- drivers/video/fbdev/core/fbmem.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/video/fbdev/core/fbmem.c +6 -0
@@ -962,6 +962,7 @@ fb_set_var(struct fb_info *info, structstruct fb_var_screeninfo old_var;struct fb_videomode mode;struct fb_event event; +u32 unused;if (var->activate &FB_ACTIVATE_INV_MODE ) {struct fb_videomode mode1, mode2;
@@ -1008,6 +1009,11 @@ fb_set_var(struct fb_info *info, structif (var->xres <8 || var->yres <8 )return -EINVAL ; +/* Too huge resolution causes multiplication overflow. */ +if (check_mul_overflow (var->xres, var->yres, &unused) || +check_mul_overflow (var->xres_virtual, var->yres_virtual, &unused)) +return -EINVAL ; + ret = info->fbops->fb_check_var (var, info);if (ret)
Greg Kroah-Hartman
maintainerPATCH 329/334 ACPI: PRM: Find PRMT table before parsing it
In reply to Greg Kroah-HartmanFrom: Aubrey Li <[email protected]>
commit 3265cc3ec52e75fc8daf189954cebda27ad26b2e upstream.
Find and verify PRMT before parsing it, which eliminates a warning on machines without PRMT:
[ 7.197173] ACPI: PRMT not present
--- drivers/acpi/prmt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/acpi/prmt.c +9 -1
@@ -288,10 +288,18 @@ invalid_guid:void __init init_prmt (void ) { +struct acpi_table_header *tbl;acpi_status status; -int mc =acpi_table_parse_entries (ACPI_SIG_PRMT ,sizeof (struct acpi_table_prmt ) + +int mc; + + status =acpi_get_table (ACPI_SIG_PRMT ,0 , &tbl); +if (ACPI_FAILURE (status)) +return ; + + mc =acpi_table_parse_entries (ACPI_SIG_PRMT ,sizeof (struct acpi_table_prmt ) +sizeof (struct acpi_table_prmt_header ),0 , acpi_parse_prmt,0 ); +acpi_put_table (tbl);/* * Return immediately if PRMT table is not present or no PRM module found. */
Greg Kroah-Hartman
maintainerPATCH 330/334 RDMA/mlx5: Fix number of allocated XLT entries
In reply to Greg Kroah-HartmanFrom: Niklas Schnelle <[email protected]>
commit 9660dcbe0d9186976917c94bce4e69dbd8d7a974 upstream.
In commit 8010d74b9965b ("RDMA/mlx5: Split the WR setup out of mlx5_ib_update_xlt()") the allocation logic was split out of mlx5_ib_update_xlt() and the logic was changed to enable better OOM handling. Sadly this change introduced a miscalculation of the number of entries that were actually allocated when under memory pressure where it can actually become 0 which on s390 lets dma_map_single() fail.
It can also lead to corruption of the free pages list when the wrong number of entries is used in the calculation of sg->length which is used as argument for free_pages().
Fix this by using the allocation size instead of misusing get_order(size).
--- drivers/infiniband/hw/mlx5/mr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/mr.c +1 -1
@@ -1024,7 +1024,7 @@ static void *mlx5_ib_alloc_xlt(size_t *nif (size >MLX5_SPARE_UMR_CHUNK ) { size =MLX5_SPARE_UMR_CHUNK ; - *nents =get_order (size) / ent_size; + *nents = size / ent_size; res = (void *)__get_free_pages (gfp_mask | __GFP_NOWARN,get_order (size));if (res)
Greg Kroah-Hartman
maintainerPATCH 331/334 bootconfig: Fix missing return check of xbc_node_compose_key function
In reply to Greg Kroah-HartmanFrom: Julio Faracco <[email protected]>
commit 903bd067faa837fddb6e5c8b740c3374dc582f04 upstream.
The function xbc_show_list should handle the keys during the composition. Even the errors returned by the compose function. Instead of removing the ret variable, it should save the value and show the exact error. This missing variable is causing a compilation issue also.
--- tools/bootconfig/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
tools/bootconfig/main.c +3 -1
@@ -111,9 +111,11 @@ static void xbc_show_list(void)char key[XBC_KEYLEN_MAX ];struct xbc_node *leaf;const char *val; +int ret;xbc_for_each_key_value (leaf, val) { -if (xbc_node_compose_key (leaf, key,XBC_KEYLEN_MAX ) <0 ) { + ret =xbc_node_compose_key (leaf, key,XBC_KEYLEN_MAX ); +if (ret <0 ) {fprintf (stderr,"Failed to compose key %d\n" , ret);break ; }
Greg Kroah-Hartman
maintainerPATCH 332/334 backlight: pwm_bl: Improve bootloader/kernel device handover
In reply to Greg Kroah-HartmanFrom: Daniel Thompson <[email protected]>
commit 79fad92f2e596f5a8dd085788a24f540263ef887 upstream.
Currently there are (at least) two problems in the way pwm_bl starts managing the enable_gpio pin. Both occur when the backlight is initially off and the driver finds the pin not already in output mode and, as a result, unconditionally switches it to output-mode and asserts the signal.
Problem 1: This could cause the backlight to flicker since, at this stage in driver initialisation, we have no idea what the PWM and regulator are doing (an unconfigured PWM could easily "rest" at 100% duty cycle).
Problem 2: This will cause us not to correctly honour the post_pwm_on_delay (which also risks flickers).
Fix this by moving the code to configure the GPIO output mode until after we have examines the handover state. That allows us to initialize enable_gpio to off if the backlight is currently off and on if the backlight is on.
--- drivers/video/backlight/pwm_bl.c | 54 ++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 26 deletions(-)
drivers/video/backlight/pwm_bl.c +28 -26
@@ -409,6 +409,33 @@ static bool pwm_backlight_is_linear(strustatic int pwm_backlight_initial_power_state (const struct pwm_bl_data *pb) {struct device_node *node = pb->dev->of_node; +bool active = true; + +/* +* If the enable GPIO is present, observable (either as input +* or output) and off then the backlight is not currently active. +* */ +if (pb->enable_gpio &&gpiod_get_value_cansleep (pb->enable_gpio) ==0 ) + active = false; + +if (!regulator_is_enabled (pb->power_supply)) + active = false; + +if (!pwm_is_enabled (pb->pwm)) + active = false; + +/* +* Synchronize the enable_gpio with the observed state of the +* hardware. +*/ +if (pb->enable_gpio) +gpiod_direction_output (pb->enable_gpio, active); + +/* +* Do not change pb->enabled here! pb->enabled essentially +* tells us if we own one of the regulator's use counts and +* right now we do not. +*/ /* Not booted with device tree or no phandle link to the node */ if (!node || !node->phandle)
@@ -420,20 +447,7 @@ static int pwm_backlight_initial_power_s* assume that another driver will enable the backlight at the * appropriate time. Therefore, if it is disabled, keep it so. */ - -/* if the enable GPIO is disabled, do not enable the backlight */ -if (pb->enable_gpio &&gpiod_get_value_cansleep (pb->enable_gpio) ==0 ) -return FB_BLANK_POWERDOWN ; - -/* The regulator is disabled, do not enable the backlight */ -if (!regulator_is_enabled (pb->power_supply)) -return FB_BLANK_POWERDOWN ; - -/* The PWM is disabled, keep it like this */ -if (!pwm_is_enabled (pb->pwm)) -return FB_BLANK_POWERDOWN ; - -return FB_BLANK_UNBLANK ; +return active ?FB_BLANK_UNBLANK :FB_BLANK_POWERDOWN ; }static int pwm_backlight_probe (struct platform_device *pdev)
@@ -486,18 +500,6 @@ static int pwm_backlight_probe(struct pl goto err_alloc; } -/* -* If the GPIO is not known to be already configured as output, that -* is, if gpiod_get_direction returns either 1 or -EINVAL, change the -* direction to output and set the GPIO as active. -* Do not force the GPIO to active when it was already output as it -* could cause backlight flickering or we would enable the backlight too -* early. Leave the decision of the initial backlight state for later. -*/ -if (pb->enable_gpio && -gpiod_get_direction (pb->enable_gpio) !=0 ) -gpiod_direction_output (pb->enable_gpio,1 ); - pb->power_supply =devm_regulator_get (&pdev->dev,"power" );if (IS_ERR (pb->power_supply)) { ret =PTR_ERR (pb->power_supply);
Greg Kroah-Hartman
maintainerPATCH 333/334 parisc: Fix unaligned-access crash in bootloader
In reply to Greg Kroah-HartmanFrom: Helge Deller <[email protected]>
commit c42813b71a06a2ff4a155aa87ac609feeab76cf3 upstream.
Kernel v5.14 has various changes to optimize unaligned memory accesses, e.g. commit 0652035a5794 ("asm-generic: unaligned: remove byteshift helpers").
Those changes triggered an unalignment-exception and thus crashed the bootloader on parisc because the unaligned "output_len" variable now suddenly was read word-wise while it was read byte-wise in the past.
Fix this issue by declaring the external output_len variable as char which then forces the compiler to generate byte-accesses.
--- arch/parisc/boot/compressed/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/parisc/boot/compressed/misc.c +1 -1
@@ -26,7 +26,7 @@extern char input_data[];extern int input_len;/* output_len is inserted by the linker possibly at an unaligned address */ -extern __le32 output_len__aligned(1) ; +extern char output_len;extern char _text, _end;extern char _bss, _ebss;extern char _startcode_end;
Greg Kroah-Hartman
maintainerPATCH 334/334 clk: kirkwood: Fix a clocking boot regression
In reply to Greg Kroah-HartmanFrom: Linus Walleij <[email protected]>
commit aaedb9e00e5400220a8871180d23a83e67f29f63 upstream.
Since a few kernel releases the Pogoplug 4 has crashed like this during boot:
Unable to handle kernel NULL pointer dereference at virtual address 00000002
(...)
[<c04116ec>] (strlen) from [<c00ead80>] (kstrdup+0x1c/0x4c)
[<c00ead80>] (kstrdup) from [<c04591d8>] (__clk_register+0x44/0x37c)
[<c04591d8>] (__clk_register) from [<c04595ec>] (clk_hw_register+0x20/0x44)
[<c04595ec>] (clk_hw_register) from [<c045bfa8>] (__clk_hw_register_mux+0x198/0x1e4) [<c045bfa8>] (__clk_hw_register_mux) from [<c045c050>] (clk_register_mux_table+0x5c/0x6c) [<c045c050>] (clk_register_mux_table) from [<c0acf3e0>] (kirkwood_clk_muxing_setup.constprop.0+0x13c/0x1ac) [<c0acf3e0>] (kirkwood_clk_muxing_setup.constprop.0) from [<c0aceae0>] (of_clk_init+0x12c/0x214) [<c0aceae0>] (of_clk_init) from [<c0ab576c>] (time_init+0x20/0x2c)
[<c0ab576c>] (time_init) from [<c0ab3d18>] (start_kernel+0x3dc/0x56c)
[<c0ab3d18>] (start_kernel) from [<00000000>] (0x0)
Code: e3130020 1afffffb e12fff1e c08a1078 (e5d03000)
This is because the "powersave" mux clock 0 was provided in an unterminated array, which is required by the loop in the driver:
/* Count, allocate, and register clock muxes */
for (n = 0; desc[n].name;)
n++;Here n will go out of bounds and then call clk_register_mux() on random memory contents after the mux clock.
Fix this by terminating the array with a blank entry.
--- drivers/clk/mvebu/kirkwood.c | 1 + 1 file changed, 1 insertion(+)
drivers/clk/mvebu/kirkwood.c +1 -0
@@ -265,6 +265,7 @@ static const char *powersave_parents[] =static const struct clk_muxing_soc_desc kirkwood_mux_desc[]__initconst = { {"powersave" , powersave_parents,ARRAY_SIZE (powersave_parents),11 ,1 ,0 }, + { } }; static struct clk *clk_muxing_get_src(
Quoted 10 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
FYI,
arm clang-10, clang-11, clang-12 and clang-13 builds failed. due to this commit on 5.14 and 5.13 on following configs,
- footbridge_defconfig
- mini2440_defconfig
- s3c2410_defconfig
This was already reported on the mailing list.
ERROR: modpost: "__mulodi4" [drivers/block/nbd.ko] undefined! #1438
https://github.com/ClangBuiltLinux/linux/issues/1438
[PATCH 00/10] raise minimum GCC version to 5.1
https://lore.kernel.org/lkml/[email protected]/
linux-next: build failure while building Linus' tree
https://lore.kernel.org/all/[email protected]/
Full build log,
https://gitlab.com/Linaro/lkft/mirrors/stable/linux-stable-rc/-/jobs/1585407346#L1111
--
Linaro LKFT
https://lkft.linaro.org
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
On ARCH_BRCMSTB, using 32-bit and 64-bit ARM kernels:
Signature · 1 line
Florian
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 9 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
5.14.4-rc1 Successfully Compiled and booted on my Raspberry PI 4b (8g) (bcm2711)
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 9 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
Compiled and booted on my test system. No dmesg regressions.
thanks,
-- Shuah
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
All tests passing for Tegra ...
Test results for stable-v5.14:
10 builds: 10 pass, 0 fail
28 boots: 28 pass, 0 fail
114 tests: 114 pass, 0 failLinux version: 5.14.4-rc1-g03f7369f8044
Boards tested: tegra124-jetson-tk1, tegra186-p2771-0000,
tegra194-p2972-0000, tegra194-p3509-0000+p3668-0000,
tegra20-ventana, tegra210-p2371-2180,
tegra210-p3450-0000, tegra30-cardhu-a04Jon
Quoted 10 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
Results from Linaro’s test farm.
Regressions on arm.
ERROR: modpost: __mulodi4 [drivers/block/nbd.ko] undefined!
arm clang-10, clang-11, clang-12 and clang-13 builds failed on stable rc 5.14 and 5.13 with following arm configs,
- footbridge_defconfig
- mini2440_defconfig
- s3c2410_defconfig
## Build
* kernel: 5.14.4-rc1
* git: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
* git branch: linux-5.14.y
* git commit: 03f7369f8044e5ef52c269cbefefb634573e3cf8
* git describe: v5.14.3-335-g03f7369f8044
* test details:
https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-5.14.y/build/v5.14.3-335-g03f7369f8044
## Regressions (compared to v5.14.3-295-gfbfe8bda5718) arm clang-10, clang-11, clang-12 and clang-13 builds failed on stable rc 5.14 and 5.13 with following arm configs,
- footbridge_defconfig
- mini2440_defconfig
- s3c2410_defconfig
## No fixes (compared to v5.14.3-295-gfbfe8bda5718)
## Test result summary
total: 91273, pass: 75879, fail: 1142, skip: 13231, xfail: 1021
## Build Summary
* arc: 10 total, 10 passed, 0 failed
* arm: 289 total, 277 passed, 12 failed
* arm64: 39 total, 39 passed, 0 failed
* dragonboard-410c: 1 total, 1 passed, 0 failed
* hi6220-hikey: 1 total, 1 passed, 0 failed
* i386: 38 total, 38 passed, 0 failed
* juno-r2: 1 total, 1 passed, 0 failed
* mips: 51 total, 51 passed, 0 failed
* parisc: 12 total, 12 passed, 0 failed
* powerpc: 36 total, 35 passed, 1 failed
* riscv: 30 total, 30 passed, 0 failed
* s390: 18 total, 18 passed, 0 failed
* sh: 24 total, 24 passed, 0 failed
* sparc: 12 total, 12 passed, 0 failed
* x15: 1 total, 0 passed, 1 failed
* x86: 1 total, 1 passed, 0 failed
* x86_64: 39 total, 39 passed, 0 failed
## Test suites summary
* fwts
* igt-gpu-tools
* install-android-platform-tools-r2600
* kselftest-android
* kselftest-arm64
* kselftest-arm64/arm64.btitest.bti_c_func
* kselftest-arm64/arm64.btitest.bti_j_func
* kselftest-arm64/arm64.btitest.bti_jc_func
* kselftest-arm64/arm64.btitest.bti_none_func
* kselftest-arm64/arm64.btitest.nohint_func
* kselftest-arm64/arm64.btitest.paciasp_func
* kselftest-arm64/arm64.nobtitest.bti_c_func
* kselftest-arm64/arm64.nobtitest.bti_j_func
* kselftest-arm64/arm64.nobtitest.bti_jc_func
* kselftest-arm64/arm64.nobtitest.bti_none_func
* kselftest-arm64/arm64.nobtitest.nohint_func
* kselftest-arm64/arm64.nobtitest.paciasp_func
* kselftest-bpf
* kselftest-breakpoints
* kselftest-capabilities
* kselftest-cgroup
* kselftest-clone3
* kselftest-core
* kselftest-cpu-hotplug
* kselftest-cpufreq
* kselftest-drivers
* kselftest-efivarfs
* kselftest-filesystems
* kselftest-firmware
* kselftest-fpu
* kselftest-futex
* kselftest-gpio
* kselftest-intel_pstate
* kselftest-ipc
* kselftest-ir
* kselftest-kcmp
* kselftest-kexec
* kselftest-kvm
* kselftest-lib
* kselftest-livepatch
* kselftest-lkdtm
* kselftest-membarrier
* kselftest-memfd
* kselftest-memory-hotplug
* kselftest-mincore
* kselftest-mount
* kselftest-mqueue
* kselftest-net
* kselftest-netfilter
* kselftest-nsfs
* kselftest-openat2
* kselftest-pid_namespace
* kselftest-pidfd
* kselftest-proc
* kselftest-pstore
* kselftest-ptrace
* kselftest-rseq
* kselftest-rtc
* kselftest-seccomp
* kselftest-sigaltstack
* kselftest-size
* kselftest-splice
* kselftest-static_keys
* kselftest-sync
* kselftest-sysctl
* kselftest-tc-testing
* kselftest-timens
* kselftest-timers
* kselftest-tmpfs
* kselftest-tpm2
* kselftest-user
* kselftest-vm
* kselftest-x86
* kselftest-zram
* kunit
* kvm-unit-tests
* libgpiod
* libhugetlbfs
* linux-log-parser
* ltp-cap_bounds-tests
* ltp-commands-tests
* ltp-containers-tests
* ltp-controllers-tests
* ltp-cpuhotplug-tests
* ltp-crypto-tests
* ltp-cve-tests
* ltp-dio-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-mm-tests
* ltp-nptl-tests
* ltp-open-posix-tests
* ltp-pty-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-tracing-tests
* network-basic-tests
* packetdrill
* perf
* rcutorture
* ssuite
* timesync-off
* v4l2-compliance
--
Linaro LKFT
https://lkft.linaro.org
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
Build results:
total: 154 pass: 154 fail: 0 Qemu test results: total: 480 pass: 480 fail: 0
Guenter
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.14.4 release. There are 334 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 15 Sep 2021 13:10:21 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 9 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.14.4-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.14.y
and the diffstat can be found below.thanks,
greg k-h
Tested rc1 against the Fedora build system (aarch64, armv7, ppc64le, s390x, x86_64), and boot tested x86_64. No regressions noted.