Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[lineage-19] Fix CVE-2021-3968 #67

Merged
merged 9 commits into from
Dec 29, 2022
Merged

[lineage-19] Fix CVE-2021-3968 #67

merged 9 commits into from
Dec 29, 2022

Conversation

Flamefire
Copy link
Contributor

As described in #66 this is the minimal approach to fix CVE-2021-3968 (with 2 simple commits first for extra safety)

Upstream those are:

The IMO better approach would be to backport as many of the LSM changes as possible. I've done that for 17.1/18.1 with Flamefire#24 I can rebase that on your 19.1 branch too if you want that (see again #66)

IMPORTANT: This an untested port from the 17.1/18.1 build where this works (device boots and camera shows pictures) but I couldn't test it for 19.1 myself, so please do that first. Note that you can fetch from a pr with git fetch github pull/<pr-num>/head and access that via git checkout FETCH_HEAD (or merge, or ...)

@Flamefire Flamefire force-pushed the binder_secid_los19 branch from 8ea2685 to 5a2e5d5 Compare June 27, 2022 15:33
@derfelot
Copy link
Member

Thanks, I'll try to do a quick test build now

@derfelot
Copy link
Member

looks like something is missing:

../../../../../../kernel/sony/msm8998/drivers/android/binder.c:3139:32: error: no member named 'cred' in 'struct binder_proc'
                security_cred_getsecid(proc->cred, &secid);

@Flamefire
Copy link
Contributor Author

Yes, seems you need 1 commit (or better all 4) from Flamefire#15

BK1603 and others added 9 commits November 27, 2022 17:59
@BK1603 @Flamefire
BACKPORT: ip_gre: add validation for csum_start
c084238 
[ Upstream commit 1d011c4803c72f3907eccfc1ec63caefb852fcbf ]

Validate csum_start in gre_handle_offloads before we call _gre_xmit so
that we do not crash later when the csum_start value is used in the
lco_csum function call.

This patch deals with ipv4 code.

Fixes: c544193 ("GRE: Refactor GRE tunneling code.")
Reported-by: syzbot+ff8e1b9f2f36481e2efc@syzkaller.appspotmail.com
Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh630@gmail.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Change-Id: I9dca612e35d2fc085f183ff9bcdcfa4b9cba3d0a
Bug: 150694665
@toddkjos @Flamefire
BACKPORT: binder: use euid from cred instead of using task
fa22aa6 
commit 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b upstream.

Save the 'struct cred' associated with a binder process
at initial open to avoid potential race conditions
when converting to an euid.

Set a transaction's sender_euid from the 'struct cred'
saved at binder_open() instead of looking up the euid
from the binder proc's 'struct task'. This ensures
the euid is associated with the security context that
of the task that opened binder.

Cc: stable@vger.kernel.org # 4.4+
Fixes: 457b9a6 ("Staging: android: add binder driver")
Signed-off-by: Todd Kjos <tkjos@google.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Suggested-by: Jann Horn <jannh@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Change-Id: I91922e7f359df5901749f1b09094c3c68d45aed4
[ Fixed minor conflicts ]
Bug: 200688826
Signed-off-by: Todd Kjos <tkjos@google.com>
@toddkjos @Flamefire
BACKPORT: binder: use cred instead of task for selinux checks
860c75c 
commit 52f88693378a58094c538662ba652aff0253c4fe upstream.

Since binder was integrated with selinux, it has passed
'struct task_struct' associated with the binder_proc
to represent the source and target of transactions.
The conversion of task to SID was then done in the hook
implementations. It turns out that there are race conditions
which can result in an incorrect security context being used.

Fix by using the 'struct cred' saved during binder_open and pass
it to the selinux subsystem.

Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables)
Fixes: 79af730 ("Add security hooks to binder and implement the hooks for SELinux.")
Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Change-Id: Id7157515d2b08f11683aeb8ad9b8f1da075d34e7
[tkjos@ Fixed minor conflicts]
Bug: 200688826
Signed-off-by: Todd Kjos <tkjos@google.com>
@toddkjos @Flamefire
FROMGIT: binder: fix test regression due to sender_euid change
2d013da 
This is a partial revert of commit
29bc22ac5e5b ("binder: use euid from cred instead of using task").
Setting sender_euid using proc->cred caused some Android system test
regressions that need further investigation. It is a partial
reversion because subsequent patches rely on proc->cred.

Fixes: 29bc22ac5e5b ("binder: use euid from cred instead of using task")
Cc: stable@vger.kernel.org # 4.4+
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Change-Id: I9b1769a3510fed250bb21859ef8beebabe034c66
Link: https://lore.kernel.org/r/20211112180720.2858135-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 200688826
(cherry picked from commit c21a80ca0684ec2910344d72556c816cb8940c01
git: //git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus)
Signed-off-by: Todd Kjos <tkjos@google.com>
@Flamefire
security: introduce CONFIG_SECURITY_WRITABLE_HOOKS
55280c8 
Subsequent patches will add RO hardening to LSM hooks, however, SELinux
still needs to be able to perform runtime disablement after init to handle
architectures where init-time disablement via boot parameters is not feasible.

Introduce a new kernel configuration parameter CONFIG_SECURITY_WRITABLE_HOOKS,
and a helper macro __lsm_ro_after_init, to handle this case.

Signed-off-by: James Morris <james.l.morris@oracle.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: Kees Cook <keescook@chromium.org>
Change-Id: I2ce60bf0e97114a2999f684ae2efe31d2dca3ae9
@Flamefire
security: mark LSM hooks as __ro_after_init
afc5536 
Mark all of the registration hooks as __ro_after_init (via the
__lsm_ro_after_init macro).

Signed-off-by: James Morris <james.l.morris@oracle.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Kees Cook <keescook@chromium.org>
Change-Id: I3d76ce950824a535c651c8c1ca0e02e0783e12ce
@Flamefire
LSM: Initialize security_hook_heads upon registration.
202a251 
"struct security_hook_heads" is an array of "struct list_head"
where elements can be initialized just before registration.

There is no need to waste 350+ lines for initialization. Let's
initialize "struct security_hook_heads" just before registration.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Kees Cook <keescook@chromium.org>
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: James Morris <james.l.morris@oracle.com>
Change-Id: I114e16f4ce212dec4dbbdc4baab2f7f63d832bb1
@mjg59 @Flamefire
security: Add a cred_getsecid hook
0bcfb58 
For IMA purposes, we want to be able to obtain the prepared secid in the
bprm structure before the credentials are committed. Add a cred_getsecid
hook that makes this possible.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
@toddkjos @Flamefire
binder: use cred instead of task for getsecid
fd69e40 
Use the 'struct cred' saved at binder_open() to lookup
the security ID via security_cred_getsecid(). This
ensures that the security context that opened binder
is the one used to generate the secctx.

Cc: stable@vger.kernel.org # 5.4+
Fixes: ec74136 ("binder: create node flag to request sender's security context")
Signed-off-by: Todd Kjos <tkjos@google.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Change-Id: I8e10e5e182b17b70ccd21288d4c2590469586989
@Flamefire
Copy link
Contributor Author

I rebased this and Flamefire#15 onto your 19.1 branch so this should work now. I'll also open a PR with the same 9 commits onto lineage-20

@Flamefire Flamefire changed the title Fix CVE-2021-3968 [lineage-19] Fix CVE-2021-3968 Nov 27, 2022
@Flamefire Flamefire mentioned this pull request Nov 27, 2022
Merged
@derfelot derfelot merged commit 74e0514 into whatawurst:lineage-19.1 Dec 29, 2022
@Flamefire Flamefire deleted the binder_secid_los19 branch December 30, 2022 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Flamefire @derfelot @BK1603 @toddkjos @mjg59