Hi community.
Previously our team reported a race condition in IMA relates to LSM
based rules which would case IMA to match files that should be filtered
out under normal condition. The issue was originally analyzed and fixed
on mainstream. The patch and the discussion could be found here:
https://lore.kernel.org/all/20220921125804.59490-1-guozihua@huawei.com/
After that, we did a regression test on 4.19 LTS and the same issue
arises. Further analysis reveled that the issue is from a completely
different cause.
The cause is that selinux_audit_rule_init() would set the rule (which is
a second level pointer) to NULL immediately after called. The relevant
codes are as shown:
security/selinux/ss/services.c:
> int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
> {
> struct selinux_state *state = &selinux_state;
> struct policydb *policydb = &state->ss->policydb;
> struct selinux_audit_rule *tmprule;
> struct role_datum *roledatum;
> struct type_datum *typedatum;
> struct user_datum *userdatum;
> struct selinux_audit_rule **rule = (struct selinux_audit_rule **)vrule;
> int rc = 0;
>
> *rule = NULL;
*rule is set to NULL here, which means the rule on IMA side is also NULL.
>
> if (!state->initialized)
> return -EOPNOTSUPP;
...
> out:
> read_unlock(&state->ss->policy_rwlock);
>
> if (rc) {
> selinux_audit_rule_free(tmprule);
> tmprule = NULL;
> }
>
> *rule = tmprule;
rule is updated at the end of the function.
>
> return rc;
> }
security/integrity/ima/ima_policy.c:
> static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> const struct cred *cred, u32 secid,
> enum ima_hooks func, int mask)
> {...
> for (i = 0; i < MAX_LSM_RULES; i++) {
> int rc = 0;
> u32 osid;
> int retried = 0;
>
> if (!rule->lsm[i].rule)
> continue;
Setting rule to NULL would lead to LSM based rule matching being skipped.
> retry:
> switch (i) {
To solve this issue, there are multiple approaches we might take and I
would like some input from the community.
The first proposed solution would be to change
selinux_audit_rule_init(). Remove the set to NULL bit and update the
rule pointer with cmpxchg.
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index a9f2bc8443bd..aa74b04ccaf7 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -3297,10 +3297,9 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
> struct type_datum *typedatum;
> struct user_datum *userdatum;
> struct selinux_audit_rule **rule = (struct selinux_audit_rule **)vrule;
> + struct selinux_audit_rule *orig = rule;
> int rc = 0;
>
> - *rule = NULL;
> -
> if (!state->initialized)
> return -EOPNOTSUPP;
>
> @@ -3382,7 +3381,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
> tmprule = NULL;
> }
>
> - *rule = tmprule;
> + if (cmpxchg(rule, orig, tmprule) != orig)
> + selinux_audit_rule_free(tmprule);
>
> return rc;
> }
This solution would be an easy fix, but might influence other modules
calling selinux_audit_rule_init() directly or indirectly (on 4.19 LTS,
only auditfilter and IMA it seems). And it might be worth returning an
error code such as -EAGAIN.
Or, we can access rules via RCU, similar to what we do on 5.10. This
could means more code change and testing.
Reported-by: Huaxin Lu <luhuaxin1(a)huawei.com>
--
Best
GUO Zihua
The primary task of the onboard_usb_hub driver is to control the
power of an onboard USB hub. The driver gets the regulator from the
device tree property "vdd-supply" of the hub's DT node. Some boards
have device tree nodes for USB hubs supported by this driver, but
don't specify a "vdd-supply". This is not an error per se, it just
means that the onboard hub driver can't be used for these hubs, so
don't create platform devices for such nodes.
This change doesn't completely fix the reported regression. It
should fix it for the RPi 3 B Plus and boards with similar hub
configurations (compatible DT nodes without "vdd-supply"), boards
that actually use the onboard hub driver could still be impacted
by the race conditions discussed in that thread. Not creating the
platform devices for nodes without "vdd-supply" is the right
thing to do, independently from the race condition, which will
be fixed in future patch.
Fixes: 8bc063641ceb ("usb: misc: Add onboard_usb_hub driver")
Link: https://lore.kernel.org/r/d04bcc45-3471-4417-b30b-5cf9880d785d@i2se.com/
Reported-by: Stefan Wahren <stefan.wahren(a)i2se.com>
Signed-off-by: Matthias Kaehlcke <mka(a)chromium.org>
---
Changes in v2:
- don't create platform devices when "vdd-supply" is missing,
rather than returning an error from _find_onboard_hub()
- check for "vdd-supply" not "vdd" (Johan)
- updated subject and commit message
- added 'Link' tag (regzbot)
drivers/usb/misc/onboard_usb_hub_pdevs.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/misc/onboard_usb_hub_pdevs.c b/drivers/usb/misc/onboard_usb_hub_pdevs.c
index ed22a18f4ab7..8cea53b0907e 100644
--- a/drivers/usb/misc/onboard_usb_hub_pdevs.c
+++ b/drivers/usb/misc/onboard_usb_hub_pdevs.c
@@ -101,6 +101,19 @@ void onboard_hub_create_pdevs(struct usb_device *parent_hub, struct list_head *p
}
}
+ /*
+ * The primary task of the onboard_usb_hub driver is to control
+ * the power of an USB onboard hub. Some boards have device tree
+ * nodes for USB hubs supported by this driver, but don't
+ * specify a "vdd-supply", which is needed by the driver. This is
+ * not a DT error per se, it just means that the onboard hub
+ * driver can't be used with these nodes, so don't create a
+ * a platform device for such a node.
+ */
+ if (!of_get_property(np, "vdd-supply", NULL) &&
+ !of_get_property(npc, "vdd-supply", NULL))
+ goto node_put;
+
pdev = of_platform_device_create(np, NULL, &parent_hub->dev);
if (!pdev) {
dev_err(&parent_hub->dev,
--
2.39.0.314.g84b9a713c41-goog
During suspend and resume, the channel state needs to be saved locally.
Otherwise, the endpoint may access the channels while they were being
suspended and causing access violations.
Fix it by saving the channel state locally during suspend and resume.
Cc: <stable(a)vger.kernel.org> # 5.19
Fixes: e4b7b5f0f30a ("bus: mhi: ep: Add support for suspending and resuming channels")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam(a)linaro.org>
---
drivers/bus/mhi/ep/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 2362fcc8b32c..bcaaba97ef63 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -1122,6 +1122,7 @@ void mhi_ep_suspend_channels(struct mhi_ep_cntrl *mhi_cntrl)
dev_dbg(&mhi_chan->mhi_dev->dev, "Suspending channel\n");
/* Set channel state to SUSPENDED */
+ mhi_chan->state = MHI_CH_STATE_SUSPENDED;
tmp &= ~CHAN_CTX_CHSTATE_MASK;
tmp |= FIELD_PREP(CHAN_CTX_CHSTATE_MASK, MHI_CH_STATE_SUSPENDED);
mhi_cntrl->ch_ctx_cache[i].chcfg = cpu_to_le32(tmp);
@@ -1151,6 +1152,7 @@ void mhi_ep_resume_channels(struct mhi_ep_cntrl *mhi_cntrl)
dev_dbg(&mhi_chan->mhi_dev->dev, "Resuming channel\n");
/* Set channel state to RUNNING */
+ mhi_chan->state = MHI_CH_STATE_RUNNING;
tmp &= ~CHAN_CTX_CHSTATE_MASK;
tmp |= FIELD_PREP(CHAN_CTX_CHSTATE_MASK, MHI_CH_STATE_RUNNING);
mhi_cntrl->ch_ctx_cache[i].chcfg = cpu_to_le32(tmp);
--
2.25.1
There is a good chance that while the channel ring gets processed, the STOP
or RESET command for the channel might be received from the MHI host. In
those cases, the entire channel ring processing needs to be protected by
chan->lock to prevent the race where the corresponding channel ring might
be reset.
While at it, let's also add a sanity check to make sure that the ring is
started before processing it. Because, if the STOP/RESET command gets
processed while mhi_ep_ch_ring_worker() waited for chan->lock, the ring
would've been reset.
Cc: <stable(a)vger.kernel.org> # 5.19
Fixes: 03c0bb8ec983 ("bus: mhi: ep: Add support for processing channel rings")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam(a)linaro.org>
---
drivers/bus/mhi/ep/main.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 0bce6610ebf1..2362fcc8b32c 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -730,24 +730,37 @@ static void mhi_ep_ch_ring_worker(struct work_struct *work)
list_del(&itr->node);
ring = itr->ring;
+ chan = &mhi_cntrl->mhi_chan[ring->ch_id];
+ mutex_lock(&chan->lock);
+
+ /*
+ * The ring could've stopped while we waited to grab the (chan->lock), so do
+ * a sanity check before going further.
+ */
+ if (!ring->started) {
+ mutex_unlock(&chan->lock);
+ kfree(itr);
+ continue;
+ }
+
/* Update the write offset for the ring */
ret = mhi_ep_update_wr_offset(ring);
if (ret) {
dev_err(dev, "Error updating write offset for ring\n");
+ mutex_unlock(&chan->lock);
kfree(itr);
continue;
}
/* Sanity check to make sure there are elements in the ring */
if (ring->rd_offset == ring->wr_offset) {
+ mutex_unlock(&chan->lock);
kfree(itr);
continue;
}
el = &ring->ring_cache[ring->rd_offset];
- chan = &mhi_cntrl->mhi_chan[ring->ch_id];
- mutex_lock(&chan->lock);
dev_dbg(dev, "Processing the ring for channel (%u)\n", ring->ch_id);
ret = mhi_ep_process_ch_ring(ring, el);
if (ret) {
--
2.25.1
From: Francesco Dolcini <francesco.dolcini(a)toradex.com>
Add a fallback mechanism to handle the case in which #size-cells is set
to <0>. According to the DT binding the nand controller node should have
set it to 0 and this is not compatible with the legacy way of
specifying partitions directly as child nodes of the nand-controller node.
This fixes a boot failure on colibri-imx7 and potentially other boards.
Cc: stable(a)vger.kernel.org
Fixes: 753395ea1e45 ("ARM: dts: imx7: Fix NAND controller size-cells")
Link: https://lore.kernel.org/all/Y4dgBTGNWpM6SQXI@francesco-nb.int.toradex.com/
Signed-off-by: Francesco Dolcini <francesco.dolcini(a)toradex.com>
---
drivers/mtd/parsers/ofpart_core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/mtd/parsers/ofpart_core.c b/drivers/mtd/parsers/ofpart_core.c
index 192190c42fc8..aa3b7fa61e50 100644
--- a/drivers/mtd/parsers/ofpart_core.c
+++ b/drivers/mtd/parsers/ofpart_core.c
@@ -122,6 +122,17 @@ static int parse_fixed_partitions(struct mtd_info *master,
a_cells = of_n_addr_cells(pp);
s_cells = of_n_size_cells(pp);
+ if (s_cells == 0) {
+ /*
+ * Use #size-cells = <1> for backward compatibility
+ * in case #size-cells is set to <0> and firmware adds
+ * OF partitions without setting it.
+ */
+ pr_warn_once("%s: ofpart partition %pOF (%pOF) #size-cells is <0>, using <1> for backward compatibility.\n",
+ master->name, pp,
+ mtd_node);
+ s_cells = 1;
+ }
if (len / 4 != a_cells + s_cells) {
pr_debug("%s: ofpart partition %pOF (%pOF) error parsing reg property.\n",
master->name, pp,
--
2.25.1
The series is intended for stable(a)vger.kernel.org # 5.4+
Syzkaller reported the following bug on linux-5.{4, 10, 15}.y:
https://syzkaller.appspot.com/bug?id=ce5575575f074c33ff80d104f5baee26f22e95…
The upstream commit that introduces this bug is:
1ed1d5921139 ("net: skip virtio_net_hdr_set_proto if protocol already set")
Upstream fixes the bug with the following commits, one of which introduces
new support:
e9d3f80935b6 ("net/af_packet: make sure to pull mac header")
dfed913e8b55 ("net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO")
The additional logic and risk backported seems manageable.
The blammed commit introduces a kernel BUG in __skb_gso_segment for
AF_PACKET SOCK_RAW GSO VLAN tagged packets. What happens is that
virtio_net_hdr_set_proto() exists early as skb->protocol is already set to
ETH_P_ALL. Then in packet_parse_headers() skb->protocol is set to
ETH_P_8021AD, but neither the network header position is adjusted, nor the
mac header is pulled. Thus when we get to validate the xmit skb and enter
skb_mac_gso_segment(), skb->mac_len has value 14, but vlan_depth gets
updated to 18 after skb_network_protocol() is called. This causes the
BUG_ON from __skb_pull(skb, vlan_depth) to be hit, as the mac header has
not been pulled yet.
The fixes from upstream backported cleanly without conflicts. I updated
the commit message of the first patch to describe the problem encountered,
and added Cc, Fixes, Reported-by and Tested-by tags. For the second patch
I just added Cc to stable indicating the versions to be fixed, and added
my Tested and Signed-off-by tags.
I tested the patches on linux-5.{4, 10, 15}.y.
Eric Dumazet (1):
net/af_packet: make sure to pull mac header
Hangbin Liu (1):
net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO
net/packet/af_packet.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
--
2.34.1