* Re: null-ptr-deref in integrity_inode_free() [not found] <20210414115055.mayc4aeonsklgwks@wittgenstein> @ 2021-04-14 17:46 ` Mimi Zohar 2021-04-14 18:19 ` Christian Brauner 0 siblings, 1 reply; 4+ messages in thread From: Mimi Zohar @ 2021-04-14 17:46 UTC (permalink / raw) To: Christian Brauner, Dmitry Kasatkin Cc: linux-integrity, keyrings, linux-security-module Hi Christian, On Wed, 2021-04-14 at 13:50 +0200, Christian Brauner wrote: > Hey, > > [Resending since the previous mail somehow hasn't made it onto the list.] > > While working on a patch to port ecryptfs to use clone_private_mount() I > hit the splat in [1] with v5.12-rc6 (Without any of my changes.). To > reproduce this you can use the config in [5]. Then run the scripts [2] > and [3] in two terminals. The kernel command line was [4]: > > while true; do ./test.sh; done > while true; do ./test2.sh; done > > and wait for [1] to appear. > > The two test scripts aren't specifically designed to trigger this issue. > They were stress tests for my ecryptfs clone_private_mount() port. They > just allow to trigger this issue. > > From a very uninformed position it seemed that: > > void integrity_inode_free(struct inode *inode) > { > struct integrity_iint_cache *iint; > > if (!IS_IMA(inode)) > return; Thank you for all the details. A builtin IMA policy wasn't specified on the boot command line. Unless a custom IMA policy is loaded, it shouldn't get beyond this point. Before looking any farther, I would appreciate your confirming that you've loaded a custom IMA policy. To see if a policy has been loaded, cat /sys/kernel/security/ima/policy. thanks, Mimi > > write_lock(&integrity_iint_lock); > iint = __integrity_iint_find(inode); > ^^^^^ > >>>>>>> is somehow NULL? > > rb_erase(&iint->rb_node, &integrity_iint_tree); > write_unlock(&integrity_iint_lock); > > iint_free(iint); > } > > So I've placed a BUG_ON() in there: > > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > index 0ba01847e836..d5152b5ef517 100644 > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -152,6 +152,7 @@ void integrity_inode_free(struct inode *inode) > > write_lock(&integrity_iint_lock); > iint = __integrity_iint_find(inode); > + BUG_ON(!iint); > rb_erase(&iint->rb_node, &integrity_iint_tree); > write_unlock(&integrity_iint_lock); > > and indeed, I get: > > [ 266.308227] ------------[ cut here ]------------ > [ 266.308592] kernel BUG at security/integrity/iint.c:155! > [ 266.312120] invalid opcode: 0000 [#1] PREEMPT SMP KASAN > [ 266.313898] CPU: 0 PID: 5867 Comm: rm Tainted: G E 5.12.0-rc6-clone-private-mount-8a97af3f33b5 #380 > [ 266.317651] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009)/LXD, BIOS 0.0.0 02/06/2015 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: null-ptr-deref in integrity_inode_free() 2021-04-14 17:46 ` null-ptr-deref in integrity_inode_free() Mimi Zohar @ 2021-04-14 18:19 ` Christian Brauner 2021-04-14 20:21 ` Mimi Zohar 0 siblings, 1 reply; 4+ messages in thread From: Christian Brauner @ 2021-04-14 18:19 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-integrity, keyrings, linux-security-module On Wed, Apr 14, 2021 at 01:46:46PM -0400, Mimi Zohar wrote: > Hi Christian, > > On Wed, 2021-04-14 at 13:50 +0200, Christian Brauner wrote: > > Hey, > > > > [Resending since the previous mail somehow hasn't made it onto the list.] > > > > While working on a patch to port ecryptfs to use clone_private_mount() I > > hit the splat in [1] with v5.12-rc6 (Without any of my changes.). To > > reproduce this you can use the config in [5]. Then run the scripts [2] > > and [3] in two terminals. The kernel command line was [4]: > > > > while true; do ./test.sh; done > > while true; do ./test2.sh; done > > > > and wait for [1] to appear. > > > > The two test scripts aren't specifically designed to trigger this issue. > > They were stress tests for my ecryptfs clone_private_mount() port. They > > just allow to trigger this issue. > > > > From a very uninformed position it seemed that: > > > > void integrity_inode_free(struct inode *inode) > > { > > struct integrity_iint_cache *iint; > > > > if (!IS_IMA(inode)) > > return; > > Thank you for all the details. > > A builtin IMA policy wasn't specified on the boot command line. Unless > a custom IMA policy is loaded, it shouldn't get beyond this > point. Before looking any farther, I would appreciate your confirming > that you've loaded a custom IMA policy. To see if a policy has been > loaded, cat /sys/kernel/security/ima/policy. Ah, interesting thank you. Here's the output: sudo cat /sys/kernel/security/ima/policy dont_measure fsmagic=0x9fa0 dont_measure fsmagic=0x62656572 dont_measure fsmagic=0x64626720 dont_measure fsmagic=0x1021994 dont_measure fsmagic=0x1cd1 dont_measure fsmagic=0x42494e4d dont_measure fsmagic=0x73636673 dont_measure fsmagic=0xf97cff8c dont_measure fsmagic=0x43415d53 dont_measure fsmagic=0x27e0eb dont_measure fsmagic=0x63677270 dont_measure fsmagic=0x6e736673 dont_measure fsmagic=0xde5e81e4 measure func=MMAP_CHECK mask=MAY_EXEC measure func=BPRM_CHECK mask=MAY_EXEC measure func=FILE_CHECK mask=^MAY_READ euid=0 measure func=FILE_CHECK mask=^MAY_READ uid=0 measure func=MODULE_CHECK measure func=FIRMWARE_CHECK measure func=POLICY_CHECK Hm, note that ecryptfs is not in this list so I guess ecryptfs would be measured? Thanks for the quick reply! Christian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: null-ptr-deref in integrity_inode_free() 2021-04-14 18:19 ` Christian Brauner @ 2021-04-14 20:21 ` Mimi Zohar [not found] ` <CAHrFyr67SkruTdYnRYy+OkrEh6wqz1A=DGHZLKE2b0s-_=Cb8g@mail.gmail.com> 0 siblings, 1 reply; 4+ messages in thread From: Mimi Zohar @ 2021-04-14 20:21 UTC (permalink / raw) To: Christian Brauner Cc: Dmitry Kasatkin, linux-integrity, keyrings, linux-security-module On Wed, 2021-04-14 at 20:19 +0200, Christian Brauner wrote: > On Wed, Apr 14, 2021 at 01:46:46PM -0400, Mimi Zohar wrote: > > Hi Christian, > > > > On Wed, 2021-04-14 at 13:50 +0200, Christian Brauner wrote: > > > Hey, > > > > > > [Resending since the previous mail somehow hasn't made it onto the list.] > > > > > > While working on a patch to port ecryptfs to use clone_private_mount() I > > > hit the splat in [1] with v5.12-rc6 (Without any of my changes.). To > > > reproduce this you can use the config in [5]. Then run the scripts [2] > > > and [3] in two terminals. The kernel command line was [4]: > > > > > > while true; do ./test.sh; done > > > while true; do ./test2.sh; done > > > > > > and wait for [1] to appear. > > > > > > The two test scripts aren't specifically designed to trigger this issue. > > > They were stress tests for my ecryptfs clone_private_mount() port. They > > > just allow to trigger this issue. > > > > > > From a very uninformed position it seemed that: > > > > > > void integrity_inode_free(struct inode *inode) > > > { > > > struct integrity_iint_cache *iint; > > > > > > if (!IS_IMA(inode)) > > > return; > > > > Thank you for all the details. > > > > A builtin IMA policy wasn't specified on the boot command line. Unless > > a custom IMA policy is loaded, it shouldn't get beyond this > > point. Before looking any farther, I would appreciate your confirming > > that you've loaded a custom IMA policy. To see if a policy has been > > loaded, cat /sys/kernel/security/ima/policy. > > Ah, interesting thank you. Here's the output: > > sudo cat /sys/kernel/security/ima/policy > dont_measure fsmagic=0x9fa0 > dont_measure fsmagic=0x62656572 > dont_measure fsmagic=0x64626720 > dont_measure fsmagic=0x1021994 > dont_measure fsmagic=0x1cd1 > dont_measure fsmagic=0x42494e4d > dont_measure fsmagic=0x73636673 > dont_measure fsmagic=0xf97cff8c > dont_measure fsmagic=0x43415d53 > dont_measure fsmagic=0x27e0eb > dont_measure fsmagic=0x63677270 > dont_measure fsmagic=0x6e736673 > dont_measure fsmagic=0xde5e81e4 > measure func=MMAP_CHECK mask=MAY_EXEC > measure func=BPRM_CHECK mask=MAY_EXEC > measure func=FILE_CHECK mask=^MAY_READ euid=0 > measure func=FILE_CHECK mask=^MAY_READ uid=0 > measure func=MODULE_CHECK > measure func=FIRMWARE_CHECK > measure func=POLICY_CHECK > > Hm, note that ecryptfs is not in this list so I guess ecryptfs would be > measured? That's the "tcb" policy, which can be specified as "policy=tcb" on the boot command line. The builtin policy rules are generic and would normally be replaced with more specific policy rules based on LSM labels. Right, ecryptfs files are being measured. There's somehow a race freeing the inode. The fix is straight forward. Did you want to fix it or should I? thanks, Mimi ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <CAHrFyr67SkruTdYnRYy+OkrEh6wqz1A=DGHZLKE2b0s-_=Cb8g@mail.gmail.com>]
* Re: null-ptr-deref in integrity_inode_free() [not found] ` <CAHrFyr67SkruTdYnRYy+OkrEh6wqz1A=DGHZLKE2b0s-_=Cb8g@mail.gmail.com> @ 2021-04-14 20:36 ` Mimi Zohar 0 siblings, 0 replies; 4+ messages in thread From: Mimi Zohar @ 2021-04-14 20:36 UTC (permalink / raw) To: Christian Brauner Cc: Dmitry Kasatkin, linux-integrity, keyrings, linux-security-module On Wed, 2021-04-14 at 22:32 +0200, Christian Brauner wrote: > On Wed, Apr 14, 2021, 22:21 Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Wed, 2021-04-14 at 20:19 +0200, Christian Brauner wrote: > > > On Wed, Apr 14, 2021 at 01:46:46PM -0400, Mimi Zohar wrote: > > > > Hi Christian, > > > > > > > > On Wed, 2021-04-14 at 13:50 +0200, Christian Brauner wrote: > > > > > Hey, > > > > > > > > > > [Resending since the previous mail somehow hasn't made it onto the list.] > > > > > > > > > > While working on a patch to port ecryptfs to use clone_private_mount() I > > > > > hit the splat in [1] with v5.12-rc6 (Without any of my changes.). To > > > > > reproduce this you can use the config in [5]. Then run the scripts [2] > > > > > and [3] in two terminals. The kernel command line was [4]: > > > > > > > > > > while true; do ./test.sh; done > > > > > while true; do ./test2.sh; done > > > > > > > > > > and wait for [1] to appear. > > > > > > > > > > The two test scripts aren't specifically designed to trigger this issue. > > > > > They were stress tests for my ecryptfs clone_private_mount() port. They > > > > > just allow to trigger this issue. > > > > > > > > > > From a very uninformed position it seemed that: > > > > > > > > > > void integrity_inode_free(struct inode *inode) > > > > > { > > > > > struct integrity_iint_cache *iint; > > > > > > > > > > if (!IS_IMA(inode)) > > > > > return; > > > > > > > > Thank you for all the details. > > > > > > > > A builtin IMA policy wasn't specified on the boot command line. Unless > > > > a custom IMA policy is loaded, it shouldn't get beyond this > > > > point. Before looking any farther, I would appreciate your confirming > > > > that you've loaded a custom IMA policy. To see if a policy has been > > > > loaded, cat /sys/kernel/security/ima/policy. > > > > > > Ah, interesting thank you. Here's the output: > > > > > > sudo cat /sys/kernel/security/ima/policy > > > dont_measure fsmagic=0x9fa0 > > > dont_measure fsmagic=0x62656572 > > > dont_measure fsmagic=0x64626720 > > > dont_measure fsmagic=0x1021994 > > > dont_measure fsmagic=0x1cd1 > > > dont_measure fsmagic=0x42494e4d > > > dont_measure fsmagic=0x73636673 > > > dont_measure fsmagic=0xf97cff8c > > > dont_measure fsmagic=0x43415d53 > > > dont_measure fsmagic=0x27e0eb > > > dont_measure fsmagic=0x63677270 > > > dont_measure fsmagic=0x6e736673 > > > dont_measure fsmagic=0xde5e81e4 > > > measure func=MMAP_CHECK mask=MAY_EXEC > > > measure func=BPRM_CHECK mask=MAY_EXEC > > > measure func=FILE_CHECK mask=^MAY_READ euid=0 > > > measure func=FILE_CHECK mask=^MAY_READ uid=0 > > > measure func=MODULE_CHECK > > > measure func=FIRMWARE_CHECK > > > measure func=POLICY_CHECK > > > > > > Hm, note that ecryptfs is not in this list so I guess ecryptfs would be > > > measured? > > > > That's the "tcb" policy, which can be specified as "policy=tcb" on the > > boot command line. The builtin policy rules are generic and would > > normally be replaced with more specific policy rules based on LSM > > labels. > > Ah, thank you. > > > Right, ecryptfs files are being measured. > > > > There's somehow a race freeing the inode. The fix is straight forward. > > Did you want to fix it or should I? > > Please do fix it. I've reach eod over here. :) > You're simply going to handle the NULL case when freeing the iint? Yes, but it would be nice to understand the race and whether this is something or has been there from the beginning. Mimi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-14 20:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210414115055.mayc4aeonsklgwks@wittgenstein> 2021-04-14 17:46 ` null-ptr-deref in integrity_inode_free() Mimi Zohar 2021-04-14 18:19 ` Christian Brauner 2021-04-14 20:21 ` Mimi Zohar [not found] ` <CAHrFyr67SkruTdYnRYy+OkrEh6wqz1A=DGHZLKE2b0s-_=Cb8g@mail.gmail.com> 2021-04-14 20:36 ` Mimi Zohar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).