| Message ID | tencent_39FA49154F494DFE0FEC2F20A9A34AA9B308@qq.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | smackfs: Use rcu_assign_pointer() to ensure safe assignment in smk_set_cipso | expand |
On 9/2/2024 1:47 AM, Jiawei Ye wrote: > In the `smk_set_cipso` function, the `skp->smk_netlabel.attr.mls.cat` > field is directly assigned to a new value without using the appropriate > RCU pointer assignment functions. According to RCU usage rules, this is > illegal and can lead to unpredictable behavior, including data > inconsistencies and impossible-to-diagnose memory corruption issues. > > This possible bug was identified using a static analysis tool developed > by myself, specifically designed to detect RCU-related issues. > > To address this, the assignment is now done using rcu_assign_pointer(), > which ensures that the pointer assignment is done safely, with the > necessary memory barriers and synchronization. This change prevents > potential RCU dereference issues by ensuring that the `cat` field is > safely updated while still adhering to RCU's requirements. > > Fixes: 0817534ff9ea ("smackfs: Fix use-after-free in netlbl_catmap_walk()") > Signed-off-by: Jiawei Ye <jiawei.ye@foxmail.com> Applied to the smack-next tree. Thank you. > --- > security/smack/smackfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c > index e22aad7604e8..5dd1e164f9b1 100644 > --- a/security/smack/smackfs.c > +++ b/security/smack/smackfs.c > @@ -932,7 +932,7 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf, > } > if (rc >= 0) { > old_cat = skp->smk_netlabel.attr.mls.cat; > - skp->smk_netlabel.attr.mls.cat = ncats.attr.mls.cat; > + rcu_assign_pointer(skp->smk_netlabel.attr.mls.cat, ncats.attr.mls.cat); > skp->smk_netlabel.attr.mls.lvl = ncats.attr.mls.lvl; > synchronize_rcu(); > netlbl_catmap_free(old_cat);
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index e22aad7604e8..5dd1e164f9b1 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -932,7 +932,7 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf, } if (rc >= 0) { old_cat = skp->smk_netlabel.attr.mls.cat; - skp->smk_netlabel.attr.mls.cat = ncats.attr.mls.cat; + rcu_assign_pointer(skp->smk_netlabel.attr.mls.cat, ncats.attr.mls.cat); skp->smk_netlabel.attr.mls.lvl = ncats.attr.mls.lvl; synchronize_rcu(); netlbl_catmap_free(old_cat);
In the `smk_set_cipso` function, the `skp->smk_netlabel.attr.mls.cat` field is directly assigned to a new value without using the appropriate RCU pointer assignment functions. According to RCU usage rules, this is illegal and can lead to unpredictable behavior, including data inconsistencies and impossible-to-diagnose memory corruption issues. This possible bug was identified using a static analysis tool developed by myself, specifically designed to detect RCU-related issues. To address this, the assignment is now done using rcu_assign_pointer(), which ensures that the pointer assignment is done safely, with the necessary memory barriers and synchronization. This change prevents potential RCU dereference issues by ensuring that the `cat` field is safely updated while still adhering to RCU's requirements. Fixes: 0817534ff9ea ("smackfs: Fix use-after-free in netlbl_catmap_walk()") Signed-off-by: Jiawei Ye <jiawei.ye@foxmail.com> --- security/smack/smackfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)