| Message ID | 20210212185930.130477-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: policy load fixes | expand |
On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > even though the policy hasn't actually been reloaded. The 'out1' jump > label now becomes unused and can be removed. > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/selinuxfs.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > index 01a7d50ed39b..340711e3dc9a 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > length = sel_make_policy_nodes(fsi, newpolicy); > if (length) { > selinux_policy_cancel(fsi->state, newpolicy); > - goto out1; > + goto out; This looks good, especially with AUDIT_MAC_POLICY_LOAD recording "res=1". However, now that I'm looking at the error path here, we don't display anything if sel_make_policy_nodes() fails, do we? If security_load_policy fails we at least do a printk(), but if this fails it silently kills the policy load; at the very least I think we want a `pr_warn_ratelimited("SELinux: failed to load policy due to selinuxfs failures")` or something similar. > } > > selinux_policy_commit(fsi->state, newpolicy); > > length = count; > > -out1: > audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_POLICY_LOAD, > "auid=%u ses=%u lsm=selinux res=1", > from_kuid(&init_user_ns, audit_get_loginuid(current)), > -- > 2.29.2
On Thu, Feb 25, 2021 at 7:15 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > > even though the policy hasn't actually been reloaded. The 'out1' jump > > label now becomes unused and can be removed. > > > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > security/selinux/selinuxfs.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > > index 01a7d50ed39b..340711e3dc9a 100644 > > --- a/security/selinux/selinuxfs.c > > +++ b/security/selinux/selinuxfs.c > > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > > length = sel_make_policy_nodes(fsi, newpolicy); > > if (length) { > > selinux_policy_cancel(fsi->state, newpolicy); > > - goto out1; > > + goto out; > > This looks good, especially with AUDIT_MAC_POLICY_LOAD recording > "res=1". However, now that I'm looking at the error path here, we > don't display anything if sel_make_policy_nodes() fails, do we? If > security_load_policy fails we at least do a printk(), but if this > fails it silently kills the policy load; at the very least I think we > want a `pr_warn_ratelimited("SELinux: failed to load policy due to > selinuxfs failures")` or something similar. There are error messages in some error paths in sel_make_policy_nodes(), but not all. Those are pr_err()s, while in sel_write_load() there is a pr_warn_ratelimited(). Could we just unify the sel_make_policy_nodes() failure to a single message? (I don't think the information on which part has failed is very useful as the most likely cause here is a memory allocation failure, not bad policy.) If so, should it be a pr_warn() or pr_err()? Ratelimited or not? > > > } > > > > selinux_policy_commit(fsi->state, newpolicy); > > > > length = count; > > > > -out1: > > audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_POLICY_LOAD, > > "auid=%u ses=%u lsm=selinux res=1", > > from_kuid(&init_user_ns, audit_get_loginuid(current)), > > -- > > 2.29.2 > > -- > paul moore > www.paul-moore.com >
On Fri, Feb 26, 2021 at 9:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Thu, Feb 25, 2021 at 7:15 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > > > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > > > even though the policy hasn't actually been reloaded. The 'out1' jump > > > label now becomes unused and can be removed. > > > > > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > --- > > > security/selinux/selinuxfs.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > > > index 01a7d50ed39b..340711e3dc9a 100644 > > > --- a/security/selinux/selinuxfs.c > > > +++ b/security/selinux/selinuxfs.c > > > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > > > length = sel_make_policy_nodes(fsi, newpolicy); > > > if (length) { > > > selinux_policy_cancel(fsi->state, newpolicy); > > > - goto out1; > > > + goto out; > > > > This looks good, especially with AUDIT_MAC_POLICY_LOAD recording > > "res=1". However, now that I'm looking at the error path here, we > > don't display anything if sel_make_policy_nodes() fails, do we? If > > security_load_policy fails we at least do a printk(), but if this > > fails it silently kills the policy load; at the very least I think we > > want a `pr_warn_ratelimited("SELinux: failed to load policy due to > > selinuxfs failures")` or something similar. > > There are error messages in some error paths in > sel_make_policy_nodes(), but not all. Those are pr_err()s, while in > sel_write_load() there is a pr_warn_ratelimited(). Could we just unify > the sel_make_policy_nodes() failure to a single message? (I don't > think the information on which part has failed is very useful as the > most likely cause here is a memory allocation failure, not bad > policy.) If so, should it be a pr_warn() or pr_err()? Ratelimited or > not? My personal opinion is that the kernel only needs to provide the error details to userspace which can be useful in determining what wrong, and how the user can fix it. For example, if there is a memory allocation failure in the kernel there is often little the user can do (and it is often transient anyway due to loading and other factors), so simply reporting that there was an allocation failure while attempting X is sufficient. Beyond that, I think things can get a little fuzzy, e.g. pr_warn() or pr_err? Ratelimit or always emit the message? I also think the answers can change as userspace behaviors change over time. If one of the policy load error paths uses a pr_err() then we should probably stick with that; it also seems appropriate as failing to (re)load a SELinux policy *is* a serious matter. As far as the rate limiting is concerned, I'm not sure if that is an important difference here; if the system is getting enough requests to reload the policy, and repeatedly failing, such that the ratelimiting matters there are likely other, much larger, issues at play on the system.
On 2021-02-28 13:52:52, Paul Moore wrote: > On Fri, Feb 26, 2021 at 9:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Thu, Feb 25, 2021 at 7:15 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > > > > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > > > > even though the policy hasn't actually been reloaded. The 'out1' jump > > > > label now becomes unused and can be removed. > > > > > > > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > --- > > > > security/selinux/selinuxfs.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > > > > index 01a7d50ed39b..340711e3dc9a 100644 > > > > --- a/security/selinux/selinuxfs.c > > > > +++ b/security/selinux/selinuxfs.c > > > > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > > > > length = sel_make_policy_nodes(fsi, newpolicy); > > > > if (length) { > > > > selinux_policy_cancel(fsi->state, newpolicy); > > > > - goto out1; > > > > + goto out; > > > > > > This looks good, especially with AUDIT_MAC_POLICY_LOAD recording > > > "res=1". However, now that I'm looking at the error path here, we > > > don't display anything if sel_make_policy_nodes() fails, do we? If > > > security_load_policy fails we at least do a printk(), but if this > > > fails it silently kills the policy load; at the very least I think we > > > want a `pr_warn_ratelimited("SELinux: failed to load policy due to > > > selinuxfs failures")` or something similar. > > > > There are error messages in some error paths in > > sel_make_policy_nodes(), but not all. Those are pr_err()s, while in > > sel_write_load() there is a pr_warn_ratelimited(). Could we just unify > > the sel_make_policy_nodes() failure to a single message? (I don't > > think the information on which part has failed is very useful as the > > most likely cause here is a memory allocation failure, not bad > > policy.) If so, should it be a pr_warn() or pr_err()? Ratelimited or > > not? > > My personal opinion is that the kernel only needs to provide the error > details to userspace which can be useful in determining what wrong, > and how the user can fix it. For example, if there is a memory > allocation failure in the kernel there is often little the user can do > (and it is often transient anyway due to loading and other factors), > so simply reporting that there was an allocation failure while > attempting X is sufficient. > > Beyond that, I think things can get a little fuzzy, e.g. pr_warn() or > pr_err? Ratelimit or always emit the message? I also think the > answers can change as userspace behaviors change over time. If one of > the policy load error paths uses a pr_err() then we should probably > stick with that; it also seems appropriate as failing to (re)load a > SELinux policy *is* a serious matter. As far as the rate limiting is > concerned, I'm not sure if that is an important difference here; if > the system is getting enough requests to reload the policy, and > repeatedly failing, such that the ratelimiting matters there are > likely other, much larger, issues at play on the system. I was a little surprised to see pr_warn_ratelimited() (from both the KERN_WARNING and ratelimited perspectives) used in the policy loading error path so I poked around a bit. The description of commit 4262fb51c9f5 ("selinux: log errors when loading new policy") explains the reasoning: If the policy fails to be loaded from userspace then a warning message is printed, whereas if a failure occurs after loading policy from userspace an error message will be printed with details on where policy loading failed (recreating one of /classes/, /policy_capabilities/, /booleans/ in the SELinux fs). This seems like sound logic and would result in Ondrej using pr_err() in the sel_make_policy_nodes() error path. Tyler > > -- > paul moore > www.paul-moore.com
On Wed, Mar 3, 2021 at 3:56 AM Tyler Hicks <tyhicks@linux.microsoft.com> wrote: > On 2021-02-28 13:52:52, Paul Moore wrote: > > On Fri, Feb 26, 2021 at 9:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > On Thu, Feb 25, 2021 at 7:15 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > > > > > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > > > > > even though the policy hasn't actually been reloaded. The 'out1' jump > > > > > label now becomes unused and can be removed. > > > > > > > > > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > --- > > > > > security/selinux/selinuxfs.c | 3 +-- > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > > > > > index 01a7d50ed39b..340711e3dc9a 100644 > > > > > --- a/security/selinux/selinuxfs.c > > > > > +++ b/security/selinux/selinuxfs.c > > > > > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > > > > > length = sel_make_policy_nodes(fsi, newpolicy); > > > > > if (length) { > > > > > selinux_policy_cancel(fsi->state, newpolicy); > > > > > - goto out1; > > > > > + goto out; > > > > > > > > This looks good, especially with AUDIT_MAC_POLICY_LOAD recording > > > > "res=1". However, now that I'm looking at the error path here, we > > > > don't display anything if sel_make_policy_nodes() fails, do we? If > > > > security_load_policy fails we at least do a printk(), but if this > > > > fails it silently kills the policy load; at the very least I think we > > > > want a `pr_warn_ratelimited("SELinux: failed to load policy due to > > > > selinuxfs failures")` or something similar. > > > > > > There are error messages in some error paths in > > > sel_make_policy_nodes(), but not all. Those are pr_err()s, while in > > > sel_write_load() there is a pr_warn_ratelimited(). Could we just unify > > > the sel_make_policy_nodes() failure to a single message? (I don't > > > think the information on which part has failed is very useful as the > > > most likely cause here is a memory allocation failure, not bad > > > policy.) If so, should it be a pr_warn() or pr_err()? Ratelimited or > > > not? > > > > My personal opinion is that the kernel only needs to provide the error > > details to userspace which can be useful in determining what wrong, > > and how the user can fix it. For example, if there is a memory > > allocation failure in the kernel there is often little the user can do > > (and it is often transient anyway due to loading and other factors), > > so simply reporting that there was an allocation failure while > > attempting X is sufficient. > > > > Beyond that, I think things can get a little fuzzy, e.g. pr_warn() or > > pr_err? Ratelimit or always emit the message? I also think the > > answers can change as userspace behaviors change over time. If one of > > the policy load error paths uses a pr_err() then we should probably > > stick with that; it also seems appropriate as failing to (re)load a > > SELinux policy *is* a serious matter. As far as the rate limiting is > > concerned, I'm not sure if that is an important difference here; if > > the system is getting enough requests to reload the policy, and > > repeatedly failing, such that the ratelimiting matters there are > > likely other, much larger, issues at play on the system. > > I was a little surprised to see pr_warn_ratelimited() (from both the > KERN_WARNING and ratelimited perspectives) used in the policy loading error > path so I poked around a bit. The description of commit 4262fb51c9f5 ("selinux: > log errors when loading new policy") explains the reasoning: > > If the policy fails to be loaded from userspace then a warning message is > printed, whereas if a failure occurs after loading policy from userspace an > error message will be printed with details on where policy loading failed > (recreating one of /classes/, /policy_capabilities/, /booleans/ in the > SELinux fs). > > This seems like sound logic and would result in Ondrej using pr_err() in the > sel_make_policy_nodes() error path. The situation has changed a bit since that was written, though... Back then after the policy had been loaded there was no way to turn back and if sel_make_policy_nodes() failed, the new policy would stay and selinuxfs would have been left behind in an inconsistent/broken state. Now this issue is fixed and the new policy isn't actually applied until the selinuxfs preparation succeeds. So from a certain POV, the selinuxfs failure is no longer that fatal and could just print a warning like the other error path, because the result is the same after both failures (active policy and selinuxfs state remains unchanged). Paul (or Stephen if you are reading this and have time to comment), what do you think?
On Wed, Mar 3, 2021 at 3:54 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Wed, Mar 3, 2021 at 3:56 AM Tyler Hicks <tyhicks@linux.microsoft.com> wrote: > > On 2021-02-28 13:52:52, Paul Moore wrote: > > > On Fri, Feb 26, 2021 at 9:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > On Thu, Feb 25, 2021 at 7:15 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > On Fri, Feb 12, 2021 at 1:59 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > > > If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', > > > > > > as the latter would incorrectly log an MAC_POLICY_LOAD audit record, > > > > > > even though the policy hasn't actually been reloaded. The 'out1' jump > > > > > > label now becomes unused and can be removed. > > > > > > > > > > > > Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > > --- > > > > > > security/selinux/selinuxfs.c | 3 +-- > > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > > > > > > index 01a7d50ed39b..340711e3dc9a 100644 > > > > > > --- a/security/selinux/selinuxfs.c > > > > > > +++ b/security/selinux/selinuxfs.c > > > > > > @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, > > > > > > length = sel_make_policy_nodes(fsi, newpolicy); > > > > > > if (length) { > > > > > > selinux_policy_cancel(fsi->state, newpolicy); > > > > > > - goto out1; > > > > > > + goto out; > > > > > > > > > > This looks good, especially with AUDIT_MAC_POLICY_LOAD recording > > > > > "res=1". However, now that I'm looking at the error path here, we > > > > > don't display anything if sel_make_policy_nodes() fails, do we? If > > > > > security_load_policy fails we at least do a printk(), but if this > > > > > fails it silently kills the policy load; at the very least I think we > > > > > want a `pr_warn_ratelimited("SELinux: failed to load policy due to > > > > > selinuxfs failures")` or something similar. > > > > > > > > There are error messages in some error paths in > > > > sel_make_policy_nodes(), but not all. Those are pr_err()s, while in > > > > sel_write_load() there is a pr_warn_ratelimited(). Could we just unify > > > > the sel_make_policy_nodes() failure to a single message? (I don't > > > > think the information on which part has failed is very useful as the > > > > most likely cause here is a memory allocation failure, not bad > > > > policy.) If so, should it be a pr_warn() or pr_err()? Ratelimited or > > > > not? > > > > > > My personal opinion is that the kernel only needs to provide the error > > > details to userspace which can be useful in determining what wrong, > > > and how the user can fix it. For example, if there is a memory > > > allocation failure in the kernel there is often little the user can do > > > (and it is often transient anyway due to loading and other factors), > > > so simply reporting that there was an allocation failure while > > > attempting X is sufficient. > > > > > > Beyond that, I think things can get a little fuzzy, e.g. pr_warn() or > > > pr_err? Ratelimit or always emit the message? I also think the > > > answers can change as userspace behaviors change over time. If one of > > > the policy load error paths uses a pr_err() then we should probably > > > stick with that; it also seems appropriate as failing to (re)load a > > > SELinux policy *is* a serious matter. As far as the rate limiting is > > > concerned, I'm not sure if that is an important difference here; if > > > the system is getting enough requests to reload the policy, and > > > repeatedly failing, such that the ratelimiting matters there are > > > likely other, much larger, issues at play on the system. > > > > I was a little surprised to see pr_warn_ratelimited() (from both the > > KERN_WARNING and ratelimited perspectives) used in the policy loading error > > path so I poked around a bit. The description of commit 4262fb51c9f5 ("selinux: > > log errors when loading new policy") explains the reasoning: > > > > If the policy fails to be loaded from userspace then a warning message is > > printed, whereas if a failure occurs after loading policy from userspace an > > error message will be printed with details on where policy loading failed > > (recreating one of /classes/, /policy_capabilities/, /booleans/ in the > > SELinux fs). > > > > This seems like sound logic and would result in Ondrej using pr_err() in the > > sel_make_policy_nodes() error path. > > The situation has changed a bit since that was written, though... Back > then after the policy had been loaded there was no way to turn back > and if sel_make_policy_nodes() failed, the new policy would stay and > selinuxfs would have been left behind in an inconsistent/broken state. > Now this issue is fixed and the new policy isn't actually applied > until the selinuxfs preparation succeeds. So from a certain POV, the > selinuxfs failure is no longer that fatal and could just print a > warning like the other error path, because the result is the same > after both failures (active policy and selinuxfs state remains > unchanged). > > Paul (or Stephen if you are reading this and have time to comment), > what do you think? Sorry for the late reply, I lost this in my inbox and since I already marked the patchset as "changes requested" in patchwork it fell off my radar ... Anyway, back to your question ... it does seem like pr_warn() is the right answer here for the reasons that Ondrej mentioned above, and I personally feel it is in keeping with the original patch's intention as well; "If the policy fails to be loaded from userspace then a warning message is printed ..." However, I'm not going to lose a lot of sleep over differences between pr_warn() and pr_err() here, if someone feels strongly that it should be pr_err() and can back that up with some solid reasoning and/or precedence then so be it.
On Thu, Mar 18, 2021 at 10:48 AM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Mar 3, 2021 at 3:54 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > The situation has changed a bit since that was written, though... Back > > then after the policy had been loaded there was no way to turn back > > and if sel_make_policy_nodes() failed, the new policy would stay and > > selinuxfs would have been left behind in an inconsistent/broken state. > > Now this issue is fixed and the new policy isn't actually applied > > until the selinuxfs preparation succeeds. So from a certain POV, the > > selinuxfs failure is no longer that fatal and could just print a > > warning like the other error path, because the result is the same > > after both failures (active policy and selinuxfs state remains > > unchanged). > > > > Paul (or Stephen if you are reading this and have time to comment), > > what do you think? > > Sorry for the late reply, I lost this in my inbox and since I already > marked the patchset as "changes requested" in patchwork it fell off my > radar ... > > Anyway, back to your question ... it does seem like pr_warn() is the > right answer here for the reasons that Ondrej mentioned above, and I > personally feel it is in keeping with the original patch's intention > as well; "If the policy fails to be loaded from userspace then a > warning message is printed ..." However, I'm not going to lose a lot > of sleep over differences between pr_warn() and pr_err() here, if > someone feels strongly that it should be pr_err() and can back that up > with some solid reasoning and/or precedence then so be it. That's fine with me. FWIW, I think the rationale for using pr_warn_ratelimited() for error returns from security_load_policy() was that the failure could be entirely userspace-induced, i.e. just pass the kernel an invalid policy. The pr_err() messages on sel_make_bools/classes were in contrast entirely kernel-internal errors and could leave the system in an inconsistent state as you noted.
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 01a7d50ed39b..340711e3dc9a 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -651,14 +651,13 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, length = sel_make_policy_nodes(fsi, newpolicy); if (length) { selinux_policy_cancel(fsi->state, newpolicy); - goto out1; + goto out; } selinux_policy_commit(fsi->state, newpolicy); length = count; -out1: audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_POLICY_LOAD, "auid=%u ses=%u lsm=selinux res=1", from_kuid(&init_user_ns, audit_get_loginuid(current)),
If sel_make_policy_nodes() fails, we should jump to 'out', not 'out1', as the latter would incorrectly log an MAC_POLICY_LOAD audit record, even though the policy hasn't actually been reloaded. The 'out1' jump label now becomes unused and can be removed. Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- security/selinux/selinuxfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)