The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From a7fda04bc9b6ad9da8e19c9e6e3b1dab773d068a Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Date: Fri, 8 Oct 2021 13:37:14 +0200
Subject: [PATCH] regulator: dt-bindings: samsung,s5m8767: correct
s5m8767,pmic-buck-default-dvs-idx property
The driver was always parsing "s5m8767,pmic-buck-default-dvs-idx", not
"s5m8767,pmic-buck234-default-dvs-idx".
Cc: <stable(a)vger.kernel.org>
Fixes: 26aec009f6b6 ("regulator: add device tree support for s5m8767")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Acked-by: Rob Herring <robh(a)kernel.org>
Message-Id: <20211008113723.134648-3-krzysztof.kozlowski(a)canonical.com>
Signed-off-by: Mark Brown <broonie(a)kernel.org>
diff --git a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
index d9cff1614f7a..6cd83d920155 100644
--- a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
+++ b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
@@ -39,7 +39,7 @@ Optional properties of the main device node (the parent!):
Additional properties required if either of the optional properties are used:
- - s5m8767,pmic-buck234-default-dvs-idx: Default voltage setting selected from
+ - s5m8767,pmic-buck-default-dvs-idx: Default voltage setting selected from
the possible 8 options selectable by the dvs gpios. The value of this
property should be between 0 and 7. If not specified or if out of range, the
default value of this property is set to 0.
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From b16bef60a9112b1e6daf3afd16484eb06e7ce792 Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Date: Fri, 8 Oct 2021 13:37:13 +0200
Subject: [PATCH] regulator: s5m8767: do not use reset value as DVS voltage if
GPIO DVS is disabled
The driver and its bindings, before commit 04f9f068a619 ("regulator:
s5m8767: Modify parsing method of the voltage table of buck2/3/4") were
requiring to provide at least one safe/default voltage for DVS registers
if DVS GPIO is not being enabled.
IOW, if s5m8767,pmic-buck2-uses-gpio-dvs is missing, the
s5m8767,pmic-buck2-dvs-voltage should still be present and contain one
voltage.
This requirement was coming from driver behavior matching this condition
(none of DVS GPIO is enabled): it was always initializing the DVS
selector pins to 0 and keeping the DVS enable setting at reset value
(enabled). Therefore if none of DVS GPIO is enabled in devicetree,
driver was configuring the first DVS voltage for buck[234].
Mentioned commit 04f9f068a619 ("regulator: s5m8767: Modify parsing
method of the voltage table of buck2/3/4") broke it because DVS voltage
won't be parsed from devicetree if DVS GPIO is not enabled. After the
change, driver will configure bucks to use the register reset value as
voltage which might have unpleasant effects.
Fix this by relaxing the bindings constrain: if DVS GPIO is not enabled
in devicetree (therefore DVS voltage is also not parsed), explicitly
disable it.
Cc: <stable(a)vger.kernel.org>
Fixes: 04f9f068a619 ("regulator: s5m8767: Modify parsing method of the voltage table of buck2/3/4")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Acked-by: Rob Herring <robh(a)kernel.org>
Message-Id: <20211008113723.134648-2-krzysztof.kozlowski(a)canonical.com>
Signed-off-by: Mark Brown <broonie(a)kernel.org>
diff --git a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
index 093edda0c8df..d9cff1614f7a 100644
--- a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
+++ b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
@@ -13,6 +13,14 @@ common regulator binding documented in:
Required properties of the main device node (the parent!):
+ - s5m8767,pmic-buck-ds-gpios: GPIO specifiers for three host gpio's used
+ for selecting GPIO DVS lines. It is one-to-one mapped to dvs gpio lines.
+
+ [1] If either of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
+ property is specified, then all the eight voltage values for the
+ 's5m8767,pmic-buck[2/3/4]-dvs-voltage' should be specified.
+
+Optional properties of the main device node (the parent!):
- s5m8767,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
units for buck2 when changing voltage using gpio dvs. Refer to [1] below
for additional information.
@@ -25,19 +33,6 @@ Required properties of the main device node (the parent!):
units for buck4 when changing voltage using gpio dvs. Refer to [1] below
for additional information.
- - s5m8767,pmic-buck-ds-gpios: GPIO specifiers for three host gpio's used
- for selecting GPIO DVS lines. It is one-to-one mapped to dvs gpio lines.
-
- [1] If none of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
- property is specified, the 's5m8767,pmic-buck[2/3/4]-dvs-voltage'
- property should specify atleast one voltage level (which would be a
- safe operating voltage).
-
- If either of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
- property is specified, then all the eight voltage values for the
- 's5m8767,pmic-buck[2/3/4]-dvs-voltage' should be specified.
-
-Optional properties of the main device node (the parent!):
- s5m8767,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
- s5m8767,pmic-buck3-uses-gpio-dvs: 'buck3' can be controlled by gpio dvs.
- s5m8767,pmic-buck4-uses-gpio-dvs: 'buck4' can be controlled by gpio dvs.
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 7c111bbdc2af..35269f998210 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -850,18 +850,15 @@ static int s5m8767_pmic_probe(struct platform_device *pdev)
/* DS4 GPIO */
gpio_direction_output(pdata->buck_ds[2], 0x0);
- if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
- pdata->buck4_gpiodvs) {
- regmap_update_bits(s5m8767->iodev->regmap_pmic,
- S5M8767_REG_BUCK2CTRL, 1 << 1,
- (pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1));
- regmap_update_bits(s5m8767->iodev->regmap_pmic,
- S5M8767_REG_BUCK3CTRL, 1 << 1,
- (pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1));
- regmap_update_bits(s5m8767->iodev->regmap_pmic,
- S5M8767_REG_BUCK4CTRL, 1 << 1,
- (pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1));
- }
+ regmap_update_bits(s5m8767->iodev->regmap_pmic,
+ S5M8767_REG_BUCK2CTRL, 1 << 1,
+ (pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1));
+ regmap_update_bits(s5m8767->iodev->regmap_pmic,
+ S5M8767_REG_BUCK3CTRL, 1 << 1,
+ (pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1));
+ regmap_update_bits(s5m8767->iodev->regmap_pmic,
+ S5M8767_REG_BUCK4CTRL, 1 << 1,
+ (pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1));
/* Initialize GPIO DVS registers */
for (i = 0; i < 8; i++) {
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From db05ddf7f321634c5659a0cf7ea56594e22365f7 Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard(a)mvista.com>
Date: Mon, 20 Sep 2021 06:25:37 -0500
Subject: [PATCH] ipmi:watchdog: Set panic count to proper value on a panic
You will get two decrements when the messages on a panic are sent, not
one, since commit 2033f6858970 ("ipmi: Free receive messages when in an
oops") was added, but the watchdog code had a bug where it didn't set
the value properly.
Reported-by: Anton Lundin <glance(a)acc.umu.se>
Cc: <Stable(a)vger.kernel.org> # v5.4+
Fixes: 2033f6858970 ("ipmi: Free receive messages when in an oops")
Signed-off-by: Corey Minyard <cminyard(a)mvista.com>
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index e4ff3b50de7f..f855a9665c28 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -497,7 +497,7 @@ static void panic_halt_ipmi_heartbeat(void)
msg.cmd = IPMI_WDOG_RESET_TIMER;
msg.data = NULL;
msg.data_len = 0;
- atomic_inc(&panic_done_count);
+ atomic_add(2, &panic_done_count);
rv = ipmi_request_supply_msgs(watchdog_user,
(struct ipmi_addr *) &addr,
0,
@@ -507,7 +507,7 @@ static void panic_halt_ipmi_heartbeat(void)
&panic_halt_heartbeat_recv_msg,
1);
if (rv)
- atomic_dec(&panic_done_count);
+ atomic_sub(2, &panic_done_count);
}
static struct ipmi_smi_msg panic_halt_smi_msg = {
@@ -531,12 +531,12 @@ static void panic_halt_ipmi_set_timeout(void)
/* Wait for the messages to be free. */
while (atomic_read(&panic_done_count) != 0)
ipmi_poll_interface(watchdog_user);
- atomic_inc(&panic_done_count);
+ atomic_add(2, &panic_done_count);
rv = __ipmi_set_timeout(&panic_halt_smi_msg,
&panic_halt_recv_msg,
&send_heartbeat_now);
if (rv) {
- atomic_dec(&panic_done_count);
+ atomic_sub(2, &panic_done_count);
pr_warn("Unable to extend the watchdog timeout\n");
} else {
if (send_heartbeat_now)
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From db05ddf7f321634c5659a0cf7ea56594e22365f7 Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard(a)mvista.com>
Date: Mon, 20 Sep 2021 06:25:37 -0500
Subject: [PATCH] ipmi:watchdog: Set panic count to proper value on a panic
You will get two decrements when the messages on a panic are sent, not
one, since commit 2033f6858970 ("ipmi: Free receive messages when in an
oops") was added, but the watchdog code had a bug where it didn't set
the value properly.
Reported-by: Anton Lundin <glance(a)acc.umu.se>
Cc: <Stable(a)vger.kernel.org> # v5.4+
Fixes: 2033f6858970 ("ipmi: Free receive messages when in an oops")
Signed-off-by: Corey Minyard <cminyard(a)mvista.com>
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index e4ff3b50de7f..f855a9665c28 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -497,7 +497,7 @@ static void panic_halt_ipmi_heartbeat(void)
msg.cmd = IPMI_WDOG_RESET_TIMER;
msg.data = NULL;
msg.data_len = 0;
- atomic_inc(&panic_done_count);
+ atomic_add(2, &panic_done_count);
rv = ipmi_request_supply_msgs(watchdog_user,
(struct ipmi_addr *) &addr,
0,
@@ -507,7 +507,7 @@ static void panic_halt_ipmi_heartbeat(void)
&panic_halt_heartbeat_recv_msg,
1);
if (rv)
- atomic_dec(&panic_done_count);
+ atomic_sub(2, &panic_done_count);
}
static struct ipmi_smi_msg panic_halt_smi_msg = {
@@ -531,12 +531,12 @@ static void panic_halt_ipmi_set_timeout(void)
/* Wait for the messages to be free. */
while (atomic_read(&panic_done_count) != 0)
ipmi_poll_interface(watchdog_user);
- atomic_inc(&panic_done_count);
+ atomic_add(2, &panic_done_count);
rv = __ipmi_set_timeout(&panic_halt_smi_msg,
&panic_halt_recv_msg,
&send_heartbeat_now);
if (rv) {
- atomic_dec(&panic_done_count);
+ atomic_sub(2, &panic_done_count);
pr_warn("Unable to extend the watchdog timeout\n");
} else {
if (send_heartbeat_now)
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From cbfcd13be5cb2a07868afe67520ed181956579a7 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace(a)redhat.com>
Date: Wed, 28 Jul 2021 16:03:13 +0200
Subject: [PATCH] selinux: fix race condition when computing ocontext SIDs
Current code contains a lot of racy patterns when converting an
ocontext's context structure to an SID. This is being done in a "lazy"
fashion, such that the SID is looked up in the SID table only when it's
first needed and then cached in the "sid" field of the ocontext
structure. However, this is done without any locking or memory barriers
and is thus unsafe.
Between commits 24ed7fdae669 ("selinux: use separate table for initial
SID lookup") and 66f8e2f03c02 ("selinux: sidtab reverse lookup hash
table"), this race condition lead to an actual observable bug, because a
pointer to the shared sid field was passed directly to
sidtab_context_to_sid(), which was using this location to also store an
intermediate value, which could have been read by other threads and
interpreted as an SID. In practice this caused e.g. new mounts to get a
wrong (seemingly random) filesystem context, leading to strange denials.
This bug has been spotted in the wild at least twice, see [1] and [2].
Fix the race condition by making all the racy functions use a common
helper that ensures the ocontext::sid accesses are made safely using the
appropriate SMP constructs.
Note that security_netif_sid() was populating the sid field of both
contexts stored in the ocontext, but only the first one was actually
used. The SELinux wiki's documentation on the "netifcon" policy
statement [3] suggests that using only the first context is intentional.
I kept only the handling of the first context here, as there is really
no point in doing the SID lookup for the unused one.
I wasn't able to reproduce the bug mentioned above on any kernel that
includes commit 66f8e2f03c02, even though it has been reported that the
issue occurs with that commit, too, just less frequently. Thus, I wasn't
able to verify that this patch fixes the issue, but it makes sense to
avoid the race condition regardless.
[1] https://github.com/containers/container-selinux/issues/89
[2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.o…
[3] https://selinuxproject.org/page/NetworkStatements#netifcon
Cc: stable(a)vger.kernel.org
Cc: Xinjie Zheng <xinjie(a)google.com>
Reported-by: Sujithra Periasamy <sujithra(a)google.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
Signed-off-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e5f1b2757a83..c4931bf6f92a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2376,6 +2376,43 @@ err_policy:
return rc;
}
+/**
+ * ocontext_to_sid - Helper to safely get sid for an ocontext
+ * @sidtab: SID table
+ * @c: ocontext structure
+ * @index: index of the context entry (0 or 1)
+ * @out_sid: pointer to the resulting SID value
+ *
+ * For all ocontexts except OCON_ISID the SID fields are populated
+ * on-demand when needed. Since updating the SID value is an SMP-sensitive
+ * operation, this helper must be used to do that safely.
+ *
+ * WARNING: This function may return -ESTALE, indicating that the caller
+ * must retry the operation after re-acquiring the policy pointer!
+ */
+static int ocontext_to_sid(struct sidtab *sidtab, struct ocontext *c,
+ size_t index, u32 *out_sid)
+{
+ int rc;
+ u32 sid;
+
+ /* Ensure the associated sidtab entry is visible to this thread. */
+ sid = smp_load_acquire(&c->sid[index]);
+ if (!sid) {
+ rc = sidtab_context_to_sid(sidtab, &c->context[index], &sid);
+ if (rc)
+ return rc;
+
+ /*
+ * Ensure the new sidtab entry is visible to other threads
+ * when they see the SID.
+ */
+ smp_store_release(&c->sid[index], sid);
+ }
+ *out_sid = sid;
+ return 0;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @state: SELinux state
@@ -2414,17 +2451,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_PORT;
}
@@ -2473,18 +2506,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2533,17 +2561,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2587,25 +2611,13 @@ retry:
}
if (c) {
- if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
- rc = sidtab_context_to_sid(sidtab, &c->context[1],
- &c->sid[1]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, if_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *if_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*if_sid = SECINITSID_NETIF;
@@ -2697,18 +2709,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_NODE;
}
@@ -2873,7 +2880,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
u16 sclass;
struct genfs *genfs;
struct ocontext *c;
- int rc, cmp = 0;
+ int cmp = 0;
while (path[0] == '/' && path[1] == '/')
path++;
@@ -2887,9 +2894,8 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!genfs || cmp)
- goto out;
+ return -ENOENT;
for (c = genfs->head; c; c = c->next) {
len = strlen(c->u.name);
@@ -2898,20 +2904,10 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!c)
- goto out;
-
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
- if (rc)
- goto out;
- }
+ return -ENOENT;
- *sid = c->sid[0];
- rc = 0;
-out:
- return rc;
+ return ocontext_to_sid(sidtab, c, 0, sid);
}
/**
@@ -2996,17 +2992,13 @@ retry:
if (c) {
sbsec->behavior = c->v.behavior;
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, &sbsec->sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- sbsec->sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
rc = __security_genfs_sid(policy, fstype, "/",
SECCLASS_DIR, &sbsec->sid);
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From cbfcd13be5cb2a07868afe67520ed181956579a7 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace(a)redhat.com>
Date: Wed, 28 Jul 2021 16:03:13 +0200
Subject: [PATCH] selinux: fix race condition when computing ocontext SIDs
Current code contains a lot of racy patterns when converting an
ocontext's context structure to an SID. This is being done in a "lazy"
fashion, such that the SID is looked up in the SID table only when it's
first needed and then cached in the "sid" field of the ocontext
structure. However, this is done without any locking or memory barriers
and is thus unsafe.
Between commits 24ed7fdae669 ("selinux: use separate table for initial
SID lookup") and 66f8e2f03c02 ("selinux: sidtab reverse lookup hash
table"), this race condition lead to an actual observable bug, because a
pointer to the shared sid field was passed directly to
sidtab_context_to_sid(), which was using this location to also store an
intermediate value, which could have been read by other threads and
interpreted as an SID. In practice this caused e.g. new mounts to get a
wrong (seemingly random) filesystem context, leading to strange denials.
This bug has been spotted in the wild at least twice, see [1] and [2].
Fix the race condition by making all the racy functions use a common
helper that ensures the ocontext::sid accesses are made safely using the
appropriate SMP constructs.
Note that security_netif_sid() was populating the sid field of both
contexts stored in the ocontext, but only the first one was actually
used. The SELinux wiki's documentation on the "netifcon" policy
statement [3] suggests that using only the first context is intentional.
I kept only the handling of the first context here, as there is really
no point in doing the SID lookup for the unused one.
I wasn't able to reproduce the bug mentioned above on any kernel that
includes commit 66f8e2f03c02, even though it has been reported that the
issue occurs with that commit, too, just less frequently. Thus, I wasn't
able to verify that this patch fixes the issue, but it makes sense to
avoid the race condition regardless.
[1] https://github.com/containers/container-selinux/issues/89
[2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.o…
[3] https://selinuxproject.org/page/NetworkStatements#netifcon
Cc: stable(a)vger.kernel.org
Cc: Xinjie Zheng <xinjie(a)google.com>
Reported-by: Sujithra Periasamy <sujithra(a)google.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
Signed-off-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e5f1b2757a83..c4931bf6f92a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2376,6 +2376,43 @@ err_policy:
return rc;
}
+/**
+ * ocontext_to_sid - Helper to safely get sid for an ocontext
+ * @sidtab: SID table
+ * @c: ocontext structure
+ * @index: index of the context entry (0 or 1)
+ * @out_sid: pointer to the resulting SID value
+ *
+ * For all ocontexts except OCON_ISID the SID fields are populated
+ * on-demand when needed. Since updating the SID value is an SMP-sensitive
+ * operation, this helper must be used to do that safely.
+ *
+ * WARNING: This function may return -ESTALE, indicating that the caller
+ * must retry the operation after re-acquiring the policy pointer!
+ */
+static int ocontext_to_sid(struct sidtab *sidtab, struct ocontext *c,
+ size_t index, u32 *out_sid)
+{
+ int rc;
+ u32 sid;
+
+ /* Ensure the associated sidtab entry is visible to this thread. */
+ sid = smp_load_acquire(&c->sid[index]);
+ if (!sid) {
+ rc = sidtab_context_to_sid(sidtab, &c->context[index], &sid);
+ if (rc)
+ return rc;
+
+ /*
+ * Ensure the new sidtab entry is visible to other threads
+ * when they see the SID.
+ */
+ smp_store_release(&c->sid[index], sid);
+ }
+ *out_sid = sid;
+ return 0;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @state: SELinux state
@@ -2414,17 +2451,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_PORT;
}
@@ -2473,18 +2506,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2533,17 +2561,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2587,25 +2611,13 @@ retry:
}
if (c) {
- if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
- rc = sidtab_context_to_sid(sidtab, &c->context[1],
- &c->sid[1]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, if_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *if_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*if_sid = SECINITSID_NETIF;
@@ -2697,18 +2709,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_NODE;
}
@@ -2873,7 +2880,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
u16 sclass;
struct genfs *genfs;
struct ocontext *c;
- int rc, cmp = 0;
+ int cmp = 0;
while (path[0] == '/' && path[1] == '/')
path++;
@@ -2887,9 +2894,8 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!genfs || cmp)
- goto out;
+ return -ENOENT;
for (c = genfs->head; c; c = c->next) {
len = strlen(c->u.name);
@@ -2898,20 +2904,10 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!c)
- goto out;
-
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
- if (rc)
- goto out;
- }
+ return -ENOENT;
- *sid = c->sid[0];
- rc = 0;
-out:
- return rc;
+ return ocontext_to_sid(sidtab, c, 0, sid);
}
/**
@@ -2996,17 +2992,13 @@ retry:
if (c) {
sbsec->behavior = c->v.behavior;
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, &sbsec->sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- sbsec->sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
rc = __security_genfs_sid(policy, fstype, "/",
SECCLASS_DIR, &sbsec->sid);
The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From cbfcd13be5cb2a07868afe67520ed181956579a7 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace(a)redhat.com>
Date: Wed, 28 Jul 2021 16:03:13 +0200
Subject: [PATCH] selinux: fix race condition when computing ocontext SIDs
Current code contains a lot of racy patterns when converting an
ocontext's context structure to an SID. This is being done in a "lazy"
fashion, such that the SID is looked up in the SID table only when it's
first needed and then cached in the "sid" field of the ocontext
structure. However, this is done without any locking or memory barriers
and is thus unsafe.
Between commits 24ed7fdae669 ("selinux: use separate table for initial
SID lookup") and 66f8e2f03c02 ("selinux: sidtab reverse lookup hash
table"), this race condition lead to an actual observable bug, because a
pointer to the shared sid field was passed directly to
sidtab_context_to_sid(), which was using this location to also store an
intermediate value, which could have been read by other threads and
interpreted as an SID. In practice this caused e.g. new mounts to get a
wrong (seemingly random) filesystem context, leading to strange denials.
This bug has been spotted in the wild at least twice, see [1] and [2].
Fix the race condition by making all the racy functions use a common
helper that ensures the ocontext::sid accesses are made safely using the
appropriate SMP constructs.
Note that security_netif_sid() was populating the sid field of both
contexts stored in the ocontext, but only the first one was actually
used. The SELinux wiki's documentation on the "netifcon" policy
statement [3] suggests that using only the first context is intentional.
I kept only the handling of the first context here, as there is really
no point in doing the SID lookup for the unused one.
I wasn't able to reproduce the bug mentioned above on any kernel that
includes commit 66f8e2f03c02, even though it has been reported that the
issue occurs with that commit, too, just less frequently. Thus, I wasn't
able to verify that this patch fixes the issue, but it makes sense to
avoid the race condition regardless.
[1] https://github.com/containers/container-selinux/issues/89
[2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.o…
[3] https://selinuxproject.org/page/NetworkStatements#netifcon
Cc: stable(a)vger.kernel.org
Cc: Xinjie Zheng <xinjie(a)google.com>
Reported-by: Sujithra Periasamy <sujithra(a)google.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
Signed-off-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e5f1b2757a83..c4931bf6f92a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2376,6 +2376,43 @@ err_policy:
return rc;
}
+/**
+ * ocontext_to_sid - Helper to safely get sid for an ocontext
+ * @sidtab: SID table
+ * @c: ocontext structure
+ * @index: index of the context entry (0 or 1)
+ * @out_sid: pointer to the resulting SID value
+ *
+ * For all ocontexts except OCON_ISID the SID fields are populated
+ * on-demand when needed. Since updating the SID value is an SMP-sensitive
+ * operation, this helper must be used to do that safely.
+ *
+ * WARNING: This function may return -ESTALE, indicating that the caller
+ * must retry the operation after re-acquiring the policy pointer!
+ */
+static int ocontext_to_sid(struct sidtab *sidtab, struct ocontext *c,
+ size_t index, u32 *out_sid)
+{
+ int rc;
+ u32 sid;
+
+ /* Ensure the associated sidtab entry is visible to this thread. */
+ sid = smp_load_acquire(&c->sid[index]);
+ if (!sid) {
+ rc = sidtab_context_to_sid(sidtab, &c->context[index], &sid);
+ if (rc)
+ return rc;
+
+ /*
+ * Ensure the new sidtab entry is visible to other threads
+ * when they see the SID.
+ */
+ smp_store_release(&c->sid[index], sid);
+ }
+ *out_sid = sid;
+ return 0;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @state: SELinux state
@@ -2414,17 +2451,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_PORT;
}
@@ -2473,18 +2506,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2533,17 +2561,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2587,25 +2611,13 @@ retry:
}
if (c) {
- if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
- rc = sidtab_context_to_sid(sidtab, &c->context[1],
- &c->sid[1]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, if_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *if_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*if_sid = SECINITSID_NETIF;
@@ -2697,18 +2709,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_NODE;
}
@@ -2873,7 +2880,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
u16 sclass;
struct genfs *genfs;
struct ocontext *c;
- int rc, cmp = 0;
+ int cmp = 0;
while (path[0] == '/' && path[1] == '/')
path++;
@@ -2887,9 +2894,8 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!genfs || cmp)
- goto out;
+ return -ENOENT;
for (c = genfs->head; c; c = c->next) {
len = strlen(c->u.name);
@@ -2898,20 +2904,10 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!c)
- goto out;
-
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
- if (rc)
- goto out;
- }
+ return -ENOENT;
- *sid = c->sid[0];
- rc = 0;
-out:
- return rc;
+ return ocontext_to_sid(sidtab, c, 0, sid);
}
/**
@@ -2996,17 +2992,13 @@ retry:
if (c) {
sbsec->behavior = c->v.behavior;
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, &sbsec->sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- sbsec->sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
rc = __security_genfs_sid(policy, fstype, "/",
SECCLASS_DIR, &sbsec->sid);
The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From cbfcd13be5cb2a07868afe67520ed181956579a7 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace(a)redhat.com>
Date: Wed, 28 Jul 2021 16:03:13 +0200
Subject: [PATCH] selinux: fix race condition when computing ocontext SIDs
Current code contains a lot of racy patterns when converting an
ocontext's context structure to an SID. This is being done in a "lazy"
fashion, such that the SID is looked up in the SID table only when it's
first needed and then cached in the "sid" field of the ocontext
structure. However, this is done without any locking or memory barriers
and is thus unsafe.
Between commits 24ed7fdae669 ("selinux: use separate table for initial
SID lookup") and 66f8e2f03c02 ("selinux: sidtab reverse lookup hash
table"), this race condition lead to an actual observable bug, because a
pointer to the shared sid field was passed directly to
sidtab_context_to_sid(), which was using this location to also store an
intermediate value, which could have been read by other threads and
interpreted as an SID. In practice this caused e.g. new mounts to get a
wrong (seemingly random) filesystem context, leading to strange denials.
This bug has been spotted in the wild at least twice, see [1] and [2].
Fix the race condition by making all the racy functions use a common
helper that ensures the ocontext::sid accesses are made safely using the
appropriate SMP constructs.
Note that security_netif_sid() was populating the sid field of both
contexts stored in the ocontext, but only the first one was actually
used. The SELinux wiki's documentation on the "netifcon" policy
statement [3] suggests that using only the first context is intentional.
I kept only the handling of the first context here, as there is really
no point in doing the SID lookup for the unused one.
I wasn't able to reproduce the bug mentioned above on any kernel that
includes commit 66f8e2f03c02, even though it has been reported that the
issue occurs with that commit, too, just less frequently. Thus, I wasn't
able to verify that this patch fixes the issue, but it makes sense to
avoid the race condition regardless.
[1] https://github.com/containers/container-selinux/issues/89
[2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.o…
[3] https://selinuxproject.org/page/NetworkStatements#netifcon
Cc: stable(a)vger.kernel.org
Cc: Xinjie Zheng <xinjie(a)google.com>
Reported-by: Sujithra Periasamy <sujithra(a)google.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
Signed-off-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e5f1b2757a83..c4931bf6f92a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2376,6 +2376,43 @@ err_policy:
return rc;
}
+/**
+ * ocontext_to_sid - Helper to safely get sid for an ocontext
+ * @sidtab: SID table
+ * @c: ocontext structure
+ * @index: index of the context entry (0 or 1)
+ * @out_sid: pointer to the resulting SID value
+ *
+ * For all ocontexts except OCON_ISID the SID fields are populated
+ * on-demand when needed. Since updating the SID value is an SMP-sensitive
+ * operation, this helper must be used to do that safely.
+ *
+ * WARNING: This function may return -ESTALE, indicating that the caller
+ * must retry the operation after re-acquiring the policy pointer!
+ */
+static int ocontext_to_sid(struct sidtab *sidtab, struct ocontext *c,
+ size_t index, u32 *out_sid)
+{
+ int rc;
+ u32 sid;
+
+ /* Ensure the associated sidtab entry is visible to this thread. */
+ sid = smp_load_acquire(&c->sid[index]);
+ if (!sid) {
+ rc = sidtab_context_to_sid(sidtab, &c->context[index], &sid);
+ if (rc)
+ return rc;
+
+ /*
+ * Ensure the new sidtab entry is visible to other threads
+ * when they see the SID.
+ */
+ smp_store_release(&c->sid[index], sid);
+ }
+ *out_sid = sid;
+ return 0;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @state: SELinux state
@@ -2414,17 +2451,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_PORT;
}
@@ -2473,18 +2506,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2533,17 +2561,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2587,25 +2611,13 @@ retry:
}
if (c) {
- if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
- rc = sidtab_context_to_sid(sidtab, &c->context[1],
- &c->sid[1]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, if_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *if_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*if_sid = SECINITSID_NETIF;
@@ -2697,18 +2709,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_NODE;
}
@@ -2873,7 +2880,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
u16 sclass;
struct genfs *genfs;
struct ocontext *c;
- int rc, cmp = 0;
+ int cmp = 0;
while (path[0] == '/' && path[1] == '/')
path++;
@@ -2887,9 +2894,8 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!genfs || cmp)
- goto out;
+ return -ENOENT;
for (c = genfs->head; c; c = c->next) {
len = strlen(c->u.name);
@@ -2898,20 +2904,10 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!c)
- goto out;
-
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
- if (rc)
- goto out;
- }
+ return -ENOENT;
- *sid = c->sid[0];
- rc = 0;
-out:
- return rc;
+ return ocontext_to_sid(sidtab, c, 0, sid);
}
/**
@@ -2996,17 +2992,13 @@ retry:
if (c) {
sbsec->behavior = c->v.behavior;
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, &sbsec->sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- sbsec->sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
rc = __security_genfs_sid(policy, fstype, "/",
SECCLASS_DIR, &sbsec->sid);
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From cbfcd13be5cb2a07868afe67520ed181956579a7 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace(a)redhat.com>
Date: Wed, 28 Jul 2021 16:03:13 +0200
Subject: [PATCH] selinux: fix race condition when computing ocontext SIDs
Current code contains a lot of racy patterns when converting an
ocontext's context structure to an SID. This is being done in a "lazy"
fashion, such that the SID is looked up in the SID table only when it's
first needed and then cached in the "sid" field of the ocontext
structure. However, this is done without any locking or memory barriers
and is thus unsafe.
Between commits 24ed7fdae669 ("selinux: use separate table for initial
SID lookup") and 66f8e2f03c02 ("selinux: sidtab reverse lookup hash
table"), this race condition lead to an actual observable bug, because a
pointer to the shared sid field was passed directly to
sidtab_context_to_sid(), which was using this location to also store an
intermediate value, which could have been read by other threads and
interpreted as an SID. In practice this caused e.g. new mounts to get a
wrong (seemingly random) filesystem context, leading to strange denials.
This bug has been spotted in the wild at least twice, see [1] and [2].
Fix the race condition by making all the racy functions use a common
helper that ensures the ocontext::sid accesses are made safely using the
appropriate SMP constructs.
Note that security_netif_sid() was populating the sid field of both
contexts stored in the ocontext, but only the first one was actually
used. The SELinux wiki's documentation on the "netifcon" policy
statement [3] suggests that using only the first context is intentional.
I kept only the handling of the first context here, as there is really
no point in doing the SID lookup for the unused one.
I wasn't able to reproduce the bug mentioned above on any kernel that
includes commit 66f8e2f03c02, even though it has been reported that the
issue occurs with that commit, too, just less frequently. Thus, I wasn't
able to verify that this patch fixes the issue, but it makes sense to
avoid the race condition regardless.
[1] https://github.com/containers/container-selinux/issues/89
[2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.o…
[3] https://selinuxproject.org/page/NetworkStatements#netifcon
Cc: stable(a)vger.kernel.org
Cc: Xinjie Zheng <xinjie(a)google.com>
Reported-by: Sujithra Periasamy <sujithra(a)google.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
Signed-off-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e5f1b2757a83..c4931bf6f92a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2376,6 +2376,43 @@ err_policy:
return rc;
}
+/**
+ * ocontext_to_sid - Helper to safely get sid for an ocontext
+ * @sidtab: SID table
+ * @c: ocontext structure
+ * @index: index of the context entry (0 or 1)
+ * @out_sid: pointer to the resulting SID value
+ *
+ * For all ocontexts except OCON_ISID the SID fields are populated
+ * on-demand when needed. Since updating the SID value is an SMP-sensitive
+ * operation, this helper must be used to do that safely.
+ *
+ * WARNING: This function may return -ESTALE, indicating that the caller
+ * must retry the operation after re-acquiring the policy pointer!
+ */
+static int ocontext_to_sid(struct sidtab *sidtab, struct ocontext *c,
+ size_t index, u32 *out_sid)
+{
+ int rc;
+ u32 sid;
+
+ /* Ensure the associated sidtab entry is visible to this thread. */
+ sid = smp_load_acquire(&c->sid[index]);
+ if (!sid) {
+ rc = sidtab_context_to_sid(sidtab, &c->context[index], &sid);
+ if (rc)
+ return rc;
+
+ /*
+ * Ensure the new sidtab entry is visible to other threads
+ * when they see the SID.
+ */
+ smp_store_release(&c->sid[index], sid);
+ }
+ *out_sid = sid;
+ return 0;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @state: SELinux state
@@ -2414,17 +2451,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_PORT;
}
@@ -2473,18 +2506,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2533,17 +2561,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*out_sid = SECINITSID_UNLABELED;
@@ -2587,25 +2611,13 @@ retry:
}
if (c) {
- if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
- rc = sidtab_context_to_sid(sidtab, &c->context[1],
- &c->sid[1]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, if_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *if_sid = c->sid[0];
+ if (rc)
+ goto out;
} else
*if_sid = SECINITSID_NETIF;
@@ -2697,18 +2709,13 @@ retry:
}
if (c) {
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab,
- &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, out_sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- *out_sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
*out_sid = SECINITSID_NODE;
}
@@ -2873,7 +2880,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
u16 sclass;
struct genfs *genfs;
struct ocontext *c;
- int rc, cmp = 0;
+ int cmp = 0;
while (path[0] == '/' && path[1] == '/')
path++;
@@ -2887,9 +2894,8 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!genfs || cmp)
- goto out;
+ return -ENOENT;
for (c = genfs->head; c; c = c->next) {
len = strlen(c->u.name);
@@ -2898,20 +2904,10 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
break;
}
- rc = -ENOENT;
if (!c)
- goto out;
-
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
- if (rc)
- goto out;
- }
+ return -ENOENT;
- *sid = c->sid[0];
- rc = 0;
-out:
- return rc;
+ return ocontext_to_sid(sidtab, c, 0, sid);
}
/**
@@ -2996,17 +2992,13 @@ retry:
if (c) {
sbsec->behavior = c->v.behavior;
- if (!c->sid[0]) {
- rc = sidtab_context_to_sid(sidtab, &c->context[0],
- &c->sid[0]);
- if (rc == -ESTALE) {
- rcu_read_unlock();
- goto retry;
- }
- if (rc)
- goto out;
+ rc = ocontext_to_sid(sidtab, c, 0, &sbsec->sid);
+ if (rc == -ESTALE) {
+ rcu_read_unlock();
+ goto retry;
}
- sbsec->sid = c->sid[0];
+ if (rc)
+ goto out;
} else {
rc = __security_genfs_sid(policy, fstype, "/",
SECCLASS_DIR, &sbsec->sid);
The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From 4114978dcd24e72415276bba60ff4ff355970bbc Mon Sep 17 00:00:00 2001
From: Sean Young <sean(a)mess.org>
Date: Tue, 14 Sep 2021 16:57:46 +0200
Subject: [PATCH] media: ir_toy: prevent device from hanging during transmit
If the IR Toy is receiving IR while a transmit is done, it may end up
hanging. We can prevent this from happening by re-entering sample mode
just before issuing the transmit command.
Link: https://github.com/bengtmartensson/HarcHardware/discussions/25
Cc: stable(a)vger.kernel.org
[mchehab: renamed: s/STATE_RESET/STATE_COMMAND_NO_RESP/ ]
Signed-off-by: Sean Young <sean(a)mess.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
diff --git a/drivers/media/rc/ir_toy.c b/drivers/media/rc/ir_toy.c
index d2d9346eb8f5..71aced52248f 100644
--- a/drivers/media/rc/ir_toy.c
+++ b/drivers/media/rc/ir_toy.c
@@ -26,6 +26,7 @@ static const u8 COMMAND_VERSION[] = { 'v' };
// End transmit and repeat reset command so we exit sump mode
static const u8 COMMAND_RESET[] = { 0xff, 0xff, 0, 0, 0, 0, 0 };
static const u8 COMMAND_SMODE_ENTER[] = { 's' };
+static const u8 COMMAND_SMODE_EXIT[] = { 0 };
static const u8 COMMAND_TXSTART[] = { 0x26, 0x24, 0x25, 0x03 };
#define REPLY_XMITCOUNT 't'
@@ -317,12 +318,30 @@ static int irtoy_tx(struct rc_dev *rc, uint *txbuf, uint count)
buf[i] = cpu_to_be16(v);
}
- buf[count] = cpu_to_be16(0xffff);
+ buf[count] = 0xffff;
irtoy->tx_buf = buf;
irtoy->tx_len = size;
irtoy->emitted = 0;
+ // There is an issue where if the unit is receiving IR while the
+ // first TXSTART command is sent, the device might end up hanging
+ // with its led on. It does not respond to any command when this
+ // happens. To work around this, re-enter sample mode.
+ err = irtoy_command(irtoy, COMMAND_SMODE_EXIT,
+ sizeof(COMMAND_SMODE_EXIT), STATE_COMMAND_NO_RESP);
+ if (err) {
+ dev_err(irtoy->dev, "exit sample mode: %d\n", err);
+ return err;
+ }
+
+ err = irtoy_command(irtoy, COMMAND_SMODE_ENTER,
+ sizeof(COMMAND_SMODE_ENTER), STATE_COMMAND);
+ if (err) {
+ dev_err(irtoy->dev, "enter sample mode: %d\n", err);
+ return err;
+ }
+
err = irtoy_command(irtoy, COMMAND_TXSTART, sizeof(COMMAND_TXSTART),
STATE_TX);
kfree(buf);