Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1690768 > unrolled thread
| Started by | Kees Cook <keescook@chromium.org> |
|---|---|
| First post | 2017-07-19 00:30 +0200 |
| Last post | 2017-07-19 07:30 +0200 |
| Articles | 20 on this page of 42 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
[PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-26 06:00 +0200
Re: [PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Casey Schaufler <casey@schaufler-ca.com> - 2017-07-26 20:00 +0200
[PATCH v3 12/15] smack: Remove redundant pdeath_signal clearing Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
[PATCH v3 14/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 14/15] exec: Use sane stack rlimit under secureexec James Morris <jmorris@namei.org> - 2017-07-19 11:50 +0200
[PATCH v3 11/15] exec: Use secureexec for clearing pdeath_signal Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
[PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:20 +0200
Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 06:50 +0200
Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Andy Lutomirski <luto@kernel.org> - 2017-07-20 07:00 +0200
Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
[PATCH v3 01/15] binfmt: Introduce secureexec flag Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 01/15] binfmt: Introduce secureexec flag John Johansen <john.johansen@canonical.com> - 2017-07-19 02:10 +0200
Re: [PATCH v3 01/15] binfmt: Introduce secureexec flag Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:10 +0200
[PATCH v3 15/15] exec: Consolidate pdeath_signal clearing Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
[PATCH v3 13/15] exec: Consolidate dumpability logic Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
[PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds Andy Lutomirski <luto@kernel.org> - 2017-07-19 04:00 +0200
Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
[PATCH v3 10/15] exec: Use secureexec for setting dumpability Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
Re: [PATCH v3 10/15] exec: Use secureexec for setting dumpability Kees Cook <keescook@chromium.org> - 2017-07-26 06:00 +0200
[PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:40 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 02:10 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 02:20 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-20 03:40 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 15:50 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-20 19:10 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 22:50 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-21 17:50 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-21 19:40 +0200
Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-21 21:20 +0200
[PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 00:40 +0200
Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds John Johansen <john.johansen@canonical.com> - 2017-07-19 02:10 +0200
Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:10 +0200
Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 06:50 +0200
Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Paul Moore <paul@paul-moore.com> - 2017-07-20 02:00 +0200
Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-19 01:10 +0200
Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-19 05:30 +0200
Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 07:30 +0200
Page 2 of 3 — ← Prev page 1 [2] 3 Next page →
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-07-19 11:30 +0200 |
| Subject | Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds |
| Message-ID | <u4TsU-6zb-81@gated-at.bofh.it> |
| In reply to | #1690780 |
On Tue, 18 Jul 2017, Kees Cook wrote: > Instead of a separate function, open-code the cap_elevated test, which > lets us entirely remove bprm->cap_effective (to use the local "effective" > variable instead), and more accurately examine euid/egid changes via the > existing local "is_setid". > > Cc: Serge Hallyn <serge@hallyn.com> > Cc: Andy Lutomirski <luto@kernel.org> > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: James Morris <james.l.morris@oracle.com> -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-19 00:30 +0200 |
| Subject | [PATCH v3 10/15] exec: Use secureexec for setting dumpability |
| Message-ID | <u4Jab-8f4-41@gated-at.bofh.it> |
| In reply to | #1690768 |
The examination of "current" to decide dumpability is wrong. This was a check of and euid/uid (or egid/gid) mismatch in the existing process, not the newly created one. This appears to stretch back into even the "history.git" tree. Luckily, dumpability is later set in commit_creds(). In earlier kernel versions before creds existed, similar checks also existed late in the exec flow, covering up the mistake as far back as I could find. Note that because the commit_creds() check examines differences of euid, uid, egid, gid, and capabilities between the old and new creds, it would look like the setup_new_exec() dumpability test could be entirely removed. However, the secureexec test may cover a different set of tests (specific to the LSMs) than what commit_creds() checks for. So, fix this test to use secureexec (the removed euid tests are redundant to the commoncap secureexec checks now). Cc: David Howells <dhowells@redhat.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- fs/exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/exec.c b/fs/exec.c index f9480d3e0b82..5241c8f25f5d 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1353,7 +1353,7 @@ void setup_new_exec(struct linux_binprm * bprm) current->sas_ss_sp = current->sas_ss_size = 0; - if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid())) + if (!bprm->secureexec) set_dumpable(current->mm, SUID_DUMP_USER); else set_dumpable(current->mm, suid_dumpable); -- 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-26 06:00 +0200 |
| Subject | Re: [PATCH v3 10/15] exec: Use secureexec for setting dumpability |
| Message-ID | <u7lEm-3wP-7@gated-at.bofh.it> |
| In reply to | #1690782 |
On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote: > The examination of "current" to decide dumpability is wrong. This was a > check of and euid/uid (or egid/gid) mismatch in the existing process, > not the newly created one. This appears to stretch back into even the > "history.git" tree. Luckily, dumpability is later set in commit_creds(). > In earlier kernel versions before creds existed, similar checks also > existed late in the exec flow, covering up the mistake as far back as I > could find. > > Note that because the commit_creds() check examines differences of euid, > uid, egid, gid, and capabilities between the old and new creds, it would > look like the setup_new_exec() dumpability test could be entirely removed. > However, the secureexec test may cover a different set of tests (specific > to the LSMs) than what commit_creds() checks for. So, fix this test to > use secureexec (the removed euid tests are redundant to the commoncap > secureexec checks now). > > Cc: David Howells <dhowells@redhat.com> > Signed-off-by: Kees Cook <keescook@chromium.org> David (or anyone else), how does this (and the following undiscussed patches) look? I only have a few unreviewed patches in this series, and I'd like to get some more eyes on it. Thanks! -Kees > --- > fs/exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/exec.c b/fs/exec.c > index f9480d3e0b82..5241c8f25f5d 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1353,7 +1353,7 @@ void setup_new_exec(struct linux_binprm * bprm) > > current->sas_ss_sp = current->sas_ss_size = 0; > > - if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid())) > + if (!bprm->secureexec) > set_dumpable(current->mm, SUID_DUMP_USER); > else > set_dumpable(current->mm, suid_dumpable); > -- > 2.7.4 > -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-19 00:40 +0200 |
| Subject | [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u4JjP-8ii-3@gated-at.bofh.it> |
| In reply to | #1690768 |
The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
hook since it's dealing with the same information, and all of the details
are finalized during the first call to the bprm_set_creds hook via
prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
via bprm->called_set_creds).
Here, the test can just happen at the end of the bprm_set_creds hook,
and the bprm_secureexec hook can be dropped.
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
security/selinux/hooks.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0f1450a06b02..18038f73a2f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
/* Clear any possibly unsafe personality bits on exec: */
bprm->per_clear |= PER_CLEAR_ON_SETID;
- }
-
- return 0;
-}
-
-static int selinux_bprm_secureexec(struct linux_binprm *bprm)
-{
- const struct task_security_struct *tsec = current_security();
- u32 sid, osid;
- int atsecure = 0;
-
- sid = tsec->sid;
- osid = tsec->osid;
- if (osid != sid) {
/* Enable secure mode for SIDs transitions unless
the noatsecure permission is granted between
the two SIDs, i.e. ahp returns 0. */
- atsecure = avc_has_perm(osid, sid,
- SECCLASS_PROCESS,
- PROCESS__NOATSECURE, NULL);
+ rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
+ SECCLASS_PROCESS, PROCESS__NOATSECURE,
+ NULL);
+ bprm->secureexec |= !!rc;
}
- return !!atsecure;
+ return 0;
}
static int match_file(const void *p, struct file *file, unsigned fd)
@@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
- LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-20 02:10 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u57ct-7FH-7@gated-at.bofh.it> |
| In reply to | #1690783 |
On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
> hook since it's dealing with the same information, and all of the details
> are finalized during the first call to the bprm_set_creds hook via
> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
> via bprm->called_set_creds).
>
> Here, the test can just happen at the end of the bprm_set_creds hook,
> and the bprm_secureexec hook can be dropped.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> security/selinux/hooks.c | 24 +++++-------------------
> 1 file changed, 5 insertions(+), 19 deletions(-)
This seems reasonable in the context of the other changes.
Stephen just posted an AT_SECURE test for the selinux-testsuite on the
SELinux mailing list, it would be nice to ensure that this patchset
doesn't run afoul of that.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0f1450a06b02..18038f73a2f7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>
> /* Clear any possibly unsafe personality bits on exec: */
> bprm->per_clear |= PER_CLEAR_ON_SETID;
> - }
> -
> - return 0;
> -}
> -
> -static int selinux_bprm_secureexec(struct linux_binprm *bprm)
> -{
> - const struct task_security_struct *tsec = current_security();
> - u32 sid, osid;
> - int atsecure = 0;
> -
> - sid = tsec->sid;
> - osid = tsec->osid;
>
> - if (osid != sid) {
> /* Enable secure mode for SIDs transitions unless
> the noatsecure permission is granted between
> the two SIDs, i.e. ahp returns 0. */
> - atsecure = avc_has_perm(osid, sid,
> - SECCLASS_PROCESS,
> - PROCESS__NOATSECURE, NULL);
> + rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
> + SECCLASS_PROCESS, PROCESS__NOATSECURE,
> + NULL);
> + bprm->secureexec |= !!rc;
> }
>
> - return !!atsecure;
> + return 0;
> }
>
> static int match_file(const void *p, struct file *file, unsigned fd)
> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
> LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
> LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
> - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
>
> LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
> LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
> --
> 2.7.4
--
paul moore
www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-20 02:20 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u57ma-7KQ-1@gated-at.bofh.it> |
| In reply to | #1692296 |
On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
>> hook since it's dealing with the same information, and all of the details
>> are finalized during the first call to the bprm_set_creds hook via
>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
>> via bprm->called_set_creds).
>>
>> Here, the test can just happen at the end of the bprm_set_creds hook,
>> and the bprm_secureexec hook can be dropped.
>>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Cc: Stephen Smalley <sds@tycho.nsa.gov>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> security/selinux/hooks.c | 24 +++++-------------------
>> 1 file changed, 5 insertions(+), 19 deletions(-)
>
> This seems reasonable in the context of the other changes.
>
> Stephen just posted an AT_SECURE test for the selinux-testsuite on the
> SELinux mailing list, it would be nice to ensure that this patchset
> doesn't run afoul of that.
Quick follow-up: I just merged Stephen's test into the test suite:
* https://github.com/SELinuxProject/selinux-testsuite
> Acked-by: Paul Moore <paul@paul-moore.com>
>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 0f1450a06b02..18038f73a2f7 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>>
>> /* Clear any possibly unsafe personality bits on exec: */
>> bprm->per_clear |= PER_CLEAR_ON_SETID;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -static int selinux_bprm_secureexec(struct linux_binprm *bprm)
>> -{
>> - const struct task_security_struct *tsec = current_security();
>> - u32 sid, osid;
>> - int atsecure = 0;
>> -
>> - sid = tsec->sid;
>> - osid = tsec->osid;
>>
>> - if (osid != sid) {
>> /* Enable secure mode for SIDs transitions unless
>> the noatsecure permission is granted between
>> the two SIDs, i.e. ahp returns 0. */
>> - atsecure = avc_has_perm(osid, sid,
>> - SECCLASS_PROCESS,
>> - PROCESS__NOATSECURE, NULL);
>> + rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
>> + SECCLASS_PROCESS, PROCESS__NOATSECURE,
>> + NULL);
>> + bprm->secureexec |= !!rc;
>> }
>>
>> - return !!atsecure;
>> + return 0;
>> }
>>
>> static int match_file(const void *p, struct file *file, unsigned fd)
>> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
>> LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
>> LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
>> - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
>>
>> LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
>> LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
>> --
>> 2.7.4
>
> --
> paul moore
> www.paul-moore.com
--
paul moore
www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-20 03:40 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u58BA-51-1@gated-at.bofh.it> |
| In reply to | #1692299 |
On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote: > On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: >>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >>> hook since it's dealing with the same information, and all of the details >>> are finalized during the first call to the bprm_set_creds hook via >>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >>> via bprm->called_set_creds). >>> >>> Here, the test can just happen at the end of the bprm_set_creds hook, >>> and the bprm_secureexec hook can be dropped. >>> >>> Cc: Paul Moore <paul@paul-moore.com> >>> Cc: Stephen Smalley <sds@tycho.nsa.gov> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >>> security/selinux/hooks.c | 24 +++++------------------- >>> 1 file changed, 5 insertions(+), 19 deletions(-) >> >> This seems reasonable in the context of the other changes. >> >> Stephen just posted an AT_SECURE test for the selinux-testsuite on the >> SELinux mailing list, it would be nice to ensure that this patchset >> doesn't run afoul of that. > > Quick follow-up: I just merged Stephen's test into the test suite: > > * https://github.com/SELinuxProject/selinux-testsuite Is there a quick how-to on just running the AT_SECURE test? -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-20 15:50 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5k02-8cN-27@gated-at.bofh.it> |
| In reply to | #1692334 |
On Wed, Jul 19, 2017 at 9:37 PM, Kees Cook <keescook@chromium.org> wrote: > On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote: >>> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: >>>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >>>> hook since it's dealing with the same information, and all of the details >>>> are finalized during the first call to the bprm_set_creds hook via >>>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >>>> via bprm->called_set_creds). >>>> >>>> Here, the test can just happen at the end of the bprm_set_creds hook, >>>> and the bprm_secureexec hook can be dropped. >>>> >>>> Cc: Paul Moore <paul@paul-moore.com> >>>> Cc: Stephen Smalley <sds@tycho.nsa.gov> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>> --- >>>> security/selinux/hooks.c | 24 +++++------------------- >>>> 1 file changed, 5 insertions(+), 19 deletions(-) >>> >>> This seems reasonable in the context of the other changes. >>> >>> Stephen just posted an AT_SECURE test for the selinux-testsuite on the >>> SELinux mailing list, it would be nice to ensure that this patchset >>> doesn't run afoul of that. >> >> Quick follow-up: I just merged Stephen's test into the test suite: >> >> * https://github.com/SELinuxProject/selinux-testsuite > > Is there a quick how-to on just running the AT_SECURE test? You'll need a functional SELinux system to start, I run it against Fedora Rawhide regularly* with various development kernels, but recent stable Fedora releases should work too. Occasionally I hear of people running it on Debian, but I haven't had a Debian SELinux system in some time so I can't say for certain everything is 100% working there. Once you've gotten a working system in enforcing mode, read the README file in the test suite to install the necessary dependencies (look in the "Userland and Base Policy" section), then build the tests/policy (you should be able to skip this step, as the make dependencies will handle it, but it is nice to do it separately to make sure you have the build dependencies sorted): # make ... load the test policy # make -C policy load ... run the tests: # cd tests/atsecure # ./test ... optionally uninstall the test policy: # make -C policy unload In some ways it is easier to just run the entire test suite: # make # make test Alternatively, if you've got a fairly recent git repo with all the patches merged I can build a test kernel and give it a shot for you, although fair warning it may take a day or two for me to get to it. * It is worth noting that the current 4.13-rcX releases have two bugs that affect the selinux-testsuite. The worst is a kernel panic due to a bug in overlayfs' xattr code, there is a patch available to fix it, but as of yesterday it hadn't yet hit Linus tree (I can dig it up if you need it). The second issue related to IPsec and getting peer label information over UDP connections, I haven't had a chance to sort that out yet, but at least it isn't a kernel panic. -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-20 19:10 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5n7A-1TV-7@gated-at.bofh.it> |
| In reply to | #1692948 |
On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: > Alternatively, if you've got a fairly recent git repo with all the > patches merged I can build a test kernel and give it a shot for you, > although fair warning it may take a day or two for me to get to it. Hurm, I think this will take quite a bit of time for me to set up. :P If you have a chance, I'd appreciate it if you could test the series. It's currently based on v4.12: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook If it doesn't work out or takes too much time I can work on setting up the test environment next week (travelling at the moment). Thanks for the details! -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-20 22:50 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5qyu-3Q5-9@gated-at.bofh.it> |
| In reply to | #1693099 |
On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: > On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >> Alternatively, if you've got a fairly recent git repo with all the >> patches merged I can build a test kernel and give it a shot for you, >> although fair warning it may take a day or two for me to get to it. > > Hurm, I think this will take quite a bit of time for me to set up. :P > If you have a chance, I'd appreciate it if you could test the series. > It's currently based on v4.12: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook > > If it doesn't work out or takes too much time I can work on setting up > the test environment next week (travelling at the moment). Building a kernel now, in case anyone on Fedora wants to play with it, you can find it here (when it finishes): * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-21 17:50 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5IlH-6Ek-15@gated-at.bofh.it> |
| In reply to | #1693226 |
On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>> Alternatively, if you've got a fairly recent git repo with all the >>> patches merged I can build a test kernel and give it a shot for you, >>> although fair warning it may take a day or two for me to get to it. >> >> Hurm, I think this will take quite a bit of time for me to set up. :P >> If you have a chance, I'd appreciate it if you could test the series. >> It's currently based on v4.12: >> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >> >> If it doesn't work out or takes too much time I can work on setting up >> the test environment next week (travelling at the moment). > > Building a kernel now, in case anyone on Fedora wants to play with it, > you can find it here (when it finishes): > > * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 Quick follow up, the kernel above passes the selinux-testsuite atsecure test. -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-21 19:40 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5K49-7JS-3@gated-at.bofh.it> |
| In reply to | #1693807 |
On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: >> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>>> Alternatively, if you've got a fairly recent git repo with all the >>>> patches merged I can build a test kernel and give it a shot for you, >>>> although fair warning it may take a day or two for me to get to it. >>> >>> Hurm, I think this will take quite a bit of time for me to set up. :P >>> If you have a chance, I'd appreciate it if you could test the series. >>> It's currently based on v4.12: >>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >>> >>> If it doesn't work out or takes too much time I can work on setting up >>> the test environment next week (travelling at the moment). >> >> Building a kernel now, in case anyone on Fedora wants to play with it, >> you can find it here (when it finishes): >> >> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 > > Quick follow up, the kernel above passes the selinux-testsuite atsecure test. Awesome, thanks for taking the time to test it. :) Can I add your Tested-by? -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-21 21:20 +0200 |
| Subject | Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook |
| Message-ID | <u5LCV-lm-9@gated-at.bofh.it> |
| In reply to | #1693902 |
On Fri, Jul 21, 2017 at 1:37 PM, Kees Cook <keescook@chromium.org> wrote: > On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote: >> On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote: >>> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote: >>>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote: >>>>> Alternatively, if you've got a fairly recent git repo with all the >>>>> patches merged I can build a test kernel and give it a shot for you, >>>>> although fair warning it may take a day or two for me to get to it. >>>> >>>> Hurm, I think this will take quite a bit of time for me to set up. :P >>>> If you have a chance, I'd appreciate it if you could test the series. >>>> It's currently based on v4.12: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook >>>> >>>> If it doesn't work out or takes too much time I can work on setting up >>>> the test environment next week (travelling at the moment). >>> >>> Building a kernel now, in case anyone on Fedora wants to play with it, >>> you can find it here (when it finishes): >>> >>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947 >> >> Quick follow up, the kernel above passes the selinux-testsuite atsecure test. > > Awesome, thanks for taking the time to test it. :) Can I add your Tested-by? Sorry, I should have included that, here ya go: Tested-by: Paul Moore <paul@paul-moore.com> -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-19 00:40 +0200 |
| Subject | [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u4JjQ-8ii-19@gated-at.bofh.it> |
| In reply to | #1690768 |
The cred_prepared bprm flag has a misleading name. It has nothing to do
with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
been called. Rename this flag and improve its comment.
Cc: David Howells <dhowells@redhat.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <james.l.morris@oracle.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/binfmt_flat.c | 2 +-
fs/exec.c | 2 +-
include/linux/binfmts.h | 8 ++++++--
security/apparmor/domain.c | 2 +-
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
security/tomoyo/tomoyo.c | 2 +-
7 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 2edcefc0a294..a722530cc468 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
* as we're past the point of no return and are dealing with shared
* libraries.
*/
- bprm.cred_prepared = 1;
+ bprm.called_set_creds = 1;
res = prepare_binprm(&bprm);
diff --git a/fs/exec.c b/fs/exec.c
index 904199086490..925c85a45d97 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1547,7 +1547,7 @@ int prepare_binprm(struct linux_binprm *bprm)
retval = security_bprm_set_creds(bprm);
if (retval)
return retval;
- bprm->cred_prepared = 1;
+ bprm->called_set_creds = 1;
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 9508b5f83c7e..36be5a67517a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -25,8 +25,12 @@ struct linux_binprm {
struct mm_struct *mm;
unsigned long p; /* current top of mem */
unsigned int
- cred_prepared:1,/* true if creds already prepared (multiple
- * preps happen for interpreters) */
+ /*
+ * True after the bprm_set_creds hook has been called once
+ * (multiple calls can be made via prepare_binprm() for
+ * binfmt_script/misc).
+ */
+ called_set_creds:1,
cap_effective:1,/* true if has elevated effective capabilities,
* false if not; except for init which inherits
* its parent's caps anyway */
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 001e133a3c8c..878407e023e3 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -350,7 +350,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
const char *name = NULL, *info = NULL;
int error = 0;
- if (bprm->cred_prepared)
+ if (bprm->called_set_creds)
return 0;
ctx = cred_ctx(bprm->cred);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 819fd6858b49..0f1450a06b02 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2327,7 +2327,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
/* SELinux context only depends on initial program or script and not
* the script interpreter */
- if (bprm->cred_prepared)
+ if (bprm->called_set_creds)
return 0;
old_tsec = current_security();
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 658f5d8c7e76..7d4b2e221124 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -917,7 +917,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
struct superblock_smack *sbsp;
int rc;
- if (bprm->cred_prepared)
+ if (bprm->called_set_creds)
return 0;
isp = inode->i_security;
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 130b4fa4f65f..d25b705360e0 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -76,7 +76,7 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
* Do only if this function is called for the first time of an execve
* operation.
*/
- if (bprm->cred_prepared)
+ if (bprm->called_set_creds)
return 0;
#ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
/*
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | John Johansen <john.johansen@canonical.com> |
|---|---|
| Date | 2017-07-19 02:10 +0200 |
| Subject | Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u4KIV-PC-9@gated-at.bofh.it> |
| In reply to | #1690787 |
On 07/18/2017 03:25 PM, Kees Cook wrote:
> The cred_prepared bprm flag has a misleading name. It has nothing to do
> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
> been called. Rename this flag and improve its comment.
>
> Cc: David Howells <dhowells@redhat.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
looks good
Acked-by: John Johansen <john.johansen@canonical.com>
> ---
> fs/binfmt_flat.c | 2 +-
> fs/exec.c | 2 +-
> include/linux/binfmts.h | 8 ++++++--
> security/apparmor/domain.c | 2 +-
> security/selinux/hooks.c | 2 +-
> security/smack/smack_lsm.c | 2 +-
> security/tomoyo/tomoyo.c | 2 +-
> 7 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 2edcefc0a294..a722530cc468 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
> * as we're past the point of no return and are dealing with shared
> * libraries.
> */
> - bprm.cred_prepared = 1;
> + bprm.called_set_creds = 1;
>
> res = prepare_binprm(&bprm);
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 904199086490..925c85a45d97 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1547,7 +1547,7 @@ int prepare_binprm(struct linux_binprm *bprm)
> retval = security_bprm_set_creds(bprm);
> if (retval)
> return retval;
> - bprm->cred_prepared = 1;
> + bprm->called_set_creds = 1;
>
> memset(bprm->buf, 0, BINPRM_BUF_SIZE);
> return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 9508b5f83c7e..36be5a67517a 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -25,8 +25,12 @@ struct linux_binprm {
> struct mm_struct *mm;
> unsigned long p; /* current top of mem */
> unsigned int
> - cred_prepared:1,/* true if creds already prepared (multiple
> - * preps happen for interpreters) */
> + /*
> + * True after the bprm_set_creds hook has been called once
> + * (multiple calls can be made via prepare_binprm() for
> + * binfmt_script/misc).
> + */
> + called_set_creds:1,
> cap_effective:1,/* true if has elevated effective capabilities,
> * false if not; except for init which inherits
> * its parent's caps anyway */
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 001e133a3c8c..878407e023e3 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -350,7 +350,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
> const char *name = NULL, *info = NULL;
> int error = 0;
>
> - if (bprm->cred_prepared)
> + if (bprm->called_set_creds)
> return 0;
>
> ctx = cred_ctx(bprm->cred);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 819fd6858b49..0f1450a06b02 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2327,7 +2327,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>
> /* SELinux context only depends on initial program or script and not
> * the script interpreter */
> - if (bprm->cred_prepared)
> + if (bprm->called_set_creds)
> return 0;
>
> old_tsec = current_security();
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 658f5d8c7e76..7d4b2e221124 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -917,7 +917,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
> struct superblock_smack *sbsp;
> int rc;
>
> - if (bprm->cred_prepared)
> + if (bprm->called_set_creds)
> return 0;
>
> isp = inode->i_security;
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index 130b4fa4f65f..d25b705360e0 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -76,7 +76,7 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
> * Do only if this function is called for the first time of an execve
> * operation.
> */
> - if (bprm->cred_prepared)
> + if (bprm->called_set_creds)
> return 0;
> #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
> /*
>
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-07-19 03:10 +0200 |
| Subject | Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u4LEZ-1qm-9@gated-at.bofh.it> |
| In reply to | #1690787 |
On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote: > The cred_prepared bprm flag has a misleading name. It has nothing to do > with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has > been called. Rename this flag and improve its comment. > > Cc: David Howells <dhowells@redhat.com> > Cc: John Johansen <john.johansen@canonical.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: James Morris <james.l.morris@oracle.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > fs/binfmt_flat.c | 2 +- > fs/exec.c | 2 +- > include/linux/binfmts.h | 8 ++++++-- > security/apparmor/domain.c | 2 +- > security/selinux/hooks.c | 2 +- > security/smack/smack_lsm.c | 2 +- > security/tomoyo/tomoyo.c | 2 +- > 7 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > index 2edcefc0a294..a722530cc468 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs) > * as we're past the point of no return and are dealing with shared > * libraries. > */ > - bprm.cred_prepared = 1; > + bprm.called_set_creds = 1; WTF is this? It's not, strictly speaking, a bug in this patch, but it's nonsensical. Is it fixed (presuably deleted) later? Otherwise looks good.
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-07-19 06:50 +0200 |
| Subject | Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u4P5T-3vt-7@gated-at.bofh.it> |
| In reply to | #1690863 |
On Tue, Jul 18, 2017 at 6:06 PM, Andy Lutomirski <luto@kernel.org> wrote: > On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote: >> The cred_prepared bprm flag has a misleading name. It has nothing to do >> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has >> been called. Rename this flag and improve its comment. >> >> Cc: David Howells <dhowells@redhat.com> >> Cc: John Johansen <john.johansen@canonical.com> >> Cc: Paul Moore <paul@paul-moore.com> >> Cc: Stephen Smalley <sds@tycho.nsa.gov> >> Cc: Casey Schaufler <casey@schaufler-ca.com> >> Cc: James Morris <james.l.morris@oracle.com> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> fs/binfmt_flat.c | 2 +- >> fs/exec.c | 2 +- >> include/linux/binfmts.h | 8 ++++++-- >> security/apparmor/domain.c | 2 +- >> security/selinux/hooks.c | 2 +- >> security/smack/smack_lsm.c | 2 +- >> security/tomoyo/tomoyo.c | 2 +- >> 7 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c >> index 2edcefc0a294..a722530cc468 100644 >> --- a/fs/binfmt_flat.c >> +++ b/fs/binfmt_flat.c >> @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs) >> * as we're past the point of no return and are dealing with shared >> * libraries. >> */ >> - bprm.cred_prepared = 1; >> + bprm.called_set_creds = 1; > > WTF is this? It's not, strictly speaking, a bug in this patch, but > it's nonsensical. Is it fixed (presuably deleted) later? binfmt_flat looks crazy, but I haven't seen any distros that enable it. > Otherwise looks good. Thanks! -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-07-19 11:30 +0200 |
| Subject | Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u4TsS-6zb-35@gated-at.bofh.it> |
| In reply to | #1690787 |
On Tue, 18 Jul 2017, Kees Cook wrote: > The cred_prepared bprm flag has a misleading name. It has nothing to do > with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has > been called. Rename this flag and improve its comment. > > Cc: David Howells <dhowells@redhat.com> > Cc: John Johansen <john.johansen@canonical.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: James Morris <james.l.morris@oracle.com> > Signed-off-by: Kees Cook <keescook@chromium.org> Acked-by: James Morris <james.l.morris@oracle.com> -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-07-20 02:00 +0200 |
| Subject | Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds |
| Message-ID | <u572O-7mO-9@gated-at.bofh.it> |
| In reply to | #1690787 |
On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote: > The cred_prepared bprm flag has a misleading name. It has nothing to do > with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has > been called. Rename this flag and improve its comment. > > Cc: David Howells <dhowells@redhat.com> > Cc: John Johansen <john.johansen@canonical.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: James Morris <james.l.morris@oracle.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > fs/binfmt_flat.c | 2 +- > fs/exec.c | 2 +- > include/linux/binfmts.h | 8 ++++++-- > security/apparmor/domain.c | 2 +- > security/selinux/hooks.c | 2 +- > security/smack/smack_lsm.c | 2 +- > security/tomoyo/tomoyo.c | 2 +- > 7 files changed, 12 insertions(+), 8 deletions(-) Acked-by: Paul Moore <paul@paul-moore.com> -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-07-19 01:10 +0200 |
| Message-ID | <u4JMS-fj-13@gated-at.bofh.it> |
| In reply to | #1690768 |
On Tue, 18 Jul 2017, Kees Cook wrote:
>
> This series has grown... :P
Hmm. It may be bigger, but I like it a lot better. Each step now looks
fairly obvious and is well documented.
I don't love the timing of it, but I think I'd be willing to just pull
this in before rc2 as a "we need to do this sooner or later anyway and
probably mark much of it for stable" kind of thing.
But it would be really good to get people to look at the series,
particularly Andy and the LSM folks..
Linus
[toc] | [prev] | [next] | [standalone]
Page 2 of 3 — ← Prev page 1 [2] 3 Next page →
Back to top | Article view | linux.kernel
csiph-web