* UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
@ 2018-06-21 9:04 xiaolei li
2018-07-01 8:23 ` Richard Weinberger
0 siblings, 1 reply; 6+ messages in thread
From: xiaolei li @ 2018-06-21 9:04 UTC (permalink / raw)
To: richard; +Cc: leqiao.peng, casey, linux-mtd, xiaolei.li
Hi Richard,
I ever committed one patch[1] to fix ubifs assert in ubifs_xattr_set()
if Smack is enabled.
It almost always runs OK except "creating new sub-dir under a transmute
enabled dir" which is found by Mr.Leqiao Peng. For this special case,
there still has ubifs assert problem.
With patch[1], this problem can be reproduced by these steps ( assume
that the Smack has been enabled and the Smack userspace tool also
installed ):
1). Add new smack rule in which the "aaa" for process object, "bbb" for
file or directory object. "xwt" for the permission "execute, write and
transmute"
echo "aaa bbb xwt" | smackload
2). Apply the label "aaa" to current sh process. Verify the effect by
"ps -Z"
echo aaa > /proc/self/attr/current
3) Create a directory "test_dir" and assign the "bbb" label and enable
the transmute feature upon the new directory.
Verify this by "chsmack test_dir".
mkdir test_dir
chsmack -a "bbb" -t test_dir
4) Create a new directory "newDir" under the "test_dir".
mkdir test_dir/newDir
Then, ubifs assert happens, and the stack trace is:
UBIFS assert failed in __ubifs_setxattr at 284 (pid 4041)
CPU: 2 PID: 4041 Comm: mkdir Tainted: G S W 4.9.90 #1
Call trace:
[<ffffff8008089ff8>] dump_backtrace+0x0/0x1d8
[<ffffff800808a254>] show_stack+0x24/0x30
[<ffffff800845f074>] dump_stack+0x94/0xb8
[<ffffff80083cdd78>] __ubifs_setxattr+0x358/0x5f8
[<ffffff80083ce384>] ubifs_xattr_set+0x64/0x3d0
[<ffffff800826c3bc>] __vfs_setxattr+0x7c/0x98
[<ffffff8008416d64>] smack_d_instantiate+0x304/0x350
[<ffffff800840cdec>] security_d_instantiate+0x4c/0x78
[<ffffff800825ca1c>] d_instantiate+0x3c/0x70
[<ffffff80083ab158>] ubifs_mkdir+0x1d8/0x1e0
[<ffffff800824f140>] vfs_mkdir2+0x128/0x1b0
[<ffffff8008254738>] SyS_mkdirat+0x80/0xf0
[<ffffff800808378c>] __sys_trace_return+0x0/0x4
Mr. Schaufler (original developer of SMACK) has analyzed the possible
root cause:
"I have confirmed this behavior. The issue is that the inode isn't
locked during inode instantiation. It doesn't need to be, because until
instatiation is complete the inode doesn't really exist and isn't
accessible to any other task. The assertion in the ubifs code is not
appropriate. I suggest that you contact the ubifs maintainers.
Please CC me, and I will answer any questions they may have. Thank you
for the report."
Could you kindly have a look at this problem? Thanks very much!
[1] https://patchwork.kernel.org/patch/9805535/
Thanks!
Xiaolei Li
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
2018-06-21 9:04 UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled xiaolei li
@ 2018-07-01 8:23 ` Richard Weinberger
2018-07-03 6:05 ` xiaolei li
0 siblings, 1 reply; 6+ messages in thread
From: Richard Weinberger @ 2018-07-01 8:23 UTC (permalink / raw)
To: xiaolei li; +Cc: leqiao.peng, casey, linux-mtd
Xiaolei Li,
Am Donnerstag, 21. Juni 2018, 11:04:36 CEST schrieb xiaolei li:
> Hi Richard,
>
> I ever committed one patch[1] to fix ubifs assert in ubifs_xattr_set()
> if Smack is enabled.
Ohh, that patch was missed. Sorry for that.
> It almost always runs OK except "creating new sub-dir under a transmute
> enabled dir" which is found by Mr.Leqiao Peng. For this special case,
> there still has ubifs assert problem.
>
> With patch[1], this problem can be reproduced by these steps ( assume
> that the Smack has been enabled and the Smack userspace tool also
> installed ):
> 1). Add new smack rule in which the "aaa" for process object, "bbb" for
> file or directory object. "xwt" for the permission "execute, write and
> transmute"
>
> echo "aaa bbb xwt" | smackload
>
> 2). Apply the label "aaa" to current sh process. Verify the effect by
> "ps -Z"
>
> echo aaa > /proc/self/attr/current
>
> 3) Create a directory "test_dir" and assign the "bbb" label and enable
> the transmute feature upon the new directory.
> Verify this by "chsmack test_dir".
>
> mkdir test_dir
> chsmack -a "bbb" -t test_dir
>
> 4) Create a new directory "newDir" under the "test_dir".
>
> mkdir test_dir/newDir
>
>
> Then, ubifs assert happens, and the stack trace is:
>
> UBIFS assert failed in __ubifs_setxattr at 284 (pid 4041)
> CPU: 2 PID: 4041 Comm: mkdir Tainted: G S W 4.9.90 #1
> Call trace:
> [<ffffff8008089ff8>] dump_backtrace+0x0/0x1d8
> [<ffffff800808a254>] show_stack+0x24/0x30
> [<ffffff800845f074>] dump_stack+0x94/0xb8
> [<ffffff80083cdd78>] __ubifs_setxattr+0x358/0x5f8
> [<ffffff80083ce384>] ubifs_xattr_set+0x64/0x3d0
> [<ffffff800826c3bc>] __vfs_setxattr+0x7c/0x98
> [<ffffff8008416d64>] smack_d_instantiate+0x304/0x350
> [<ffffff800840cdec>] security_d_instantiate+0x4c/0x78
> [<ffffff800825ca1c>] d_instantiate+0x3c/0x70
> [<ffffff80083ab158>] ubifs_mkdir+0x1d8/0x1e0
> [<ffffff800824f140>] vfs_mkdir2+0x128/0x1b0
> [<ffffff8008254738>] SyS_mkdirat+0x80/0xf0
> [<ffffff800808378c>] __sys_trace_return+0x0/0x4
>
> Mr. Schaufler (original developer of SMACK) has analyzed the possible
> root cause:
> "I have confirmed this behavior. The issue is that the inode isn't
> locked during inode instantiation. It doesn't need to be, because until
> instatiation is complete the inode doesn't really exist and isn't
> accessible to any other task. The assertion in the ubifs code is not
> appropriate. I suggest that you contact the ubifs maintainers.
> Please CC me, and I will answer any questions they may have. Thank you
> for the report."
Yeah, this is the same case as with encrypted files. The assert makes
no sense here. Thanks for reporting!
Does your patch from 2017 still apply and work?
Thanks,
//richard
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
2018-07-01 8:23 ` Richard Weinberger
@ 2018-07-03 6:05 ` xiaolei li
2018-07-03 7:22 ` Richard Weinberger
0 siblings, 1 reply; 6+ messages in thread
From: xiaolei li @ 2018-07-03 6:05 UTC (permalink / raw)
To: Richard Weinberger; +Cc: leqiao.peng, casey, linux-mtd
Hi, Richard,
On Sun, 2018-07-01 at 10:23 +0200, Richard Weinberger wrote:
> Xiaolei Li,
>
> Am Donnerstag, 21. Juni 2018, 11:04:36 CEST schrieb xiaolei li:
> > Hi Richard,
> >
> > I ever committed one patch[1] to fix ubifs assert in ubifs_xattr_set()
> > if Smack is enabled.
>
> Ohh, that patch was missed. Sorry for that.
>
> > It almost always runs OK except "creating new sub-dir under a transmute
> > enabled dir" which is found by Mr.Leqiao Peng. For this special case,
> > there still has ubifs assert problem.
> >
> > With patch[1], this problem can be reproduced by these steps ( assume
> > that the Smack has been enabled and the Smack userspace tool also
> > installed ):
> > 1). Add new smack rule in which the "aaa" for process object, "bbb" for
> > file or directory object. "xwt" for the permission "execute, write and
> > transmute"
> >
> > echo "aaa bbb xwt" | smackload
> >
> > 2). Apply the label "aaa" to current sh process. Verify the effect by
> > "ps -Z"
> >
> > echo aaa > /proc/self/attr/current
> >
> > 3) Create a directory "test_dir" and assign the "bbb" label and enable
> > the transmute feature upon the new directory.
> > Verify this by "chsmack test_dir".
> >
> > mkdir test_dir
> > chsmack -a "bbb" -t test_dir
> >
> > 4) Create a new directory "newDir" under the "test_dir".
> >
> > mkdir test_dir/newDir
> >
> >
> > Then, ubifs assert happens, and the stack trace is:
> >
> > UBIFS assert failed in __ubifs_setxattr at 284 (pid 4041)
> > CPU: 2 PID: 4041 Comm: mkdir Tainted: G S W 4.9.90 #1
> > Call trace:
> > [<ffffff8008089ff8>] dump_backtrace+0x0/0x1d8
> > [<ffffff800808a254>] show_stack+0x24/0x30
> > [<ffffff800845f074>] dump_stack+0x94/0xb8
> > [<ffffff80083cdd78>] __ubifs_setxattr+0x358/0x5f8
> > [<ffffff80083ce384>] ubifs_xattr_set+0x64/0x3d0
> > [<ffffff800826c3bc>] __vfs_setxattr+0x7c/0x98
> > [<ffffff8008416d64>] smack_d_instantiate+0x304/0x350
> > [<ffffff800840cdec>] security_d_instantiate+0x4c/0x78
> > [<ffffff800825ca1c>] d_instantiate+0x3c/0x70
> > [<ffffff80083ab158>] ubifs_mkdir+0x1d8/0x1e0
> > [<ffffff800824f140>] vfs_mkdir2+0x128/0x1b0
> > [<ffffff8008254738>] SyS_mkdirat+0x80/0xf0
> > [<ffffff800808378c>] __sys_trace_return+0x0/0x4
> >
> > Mr. Schaufler (original developer of SMACK) has analyzed the possible
> > root cause:
> > "I have confirmed this behavior. The issue is that the inode isn't
> > locked during inode instantiation. It doesn't need to be, because until
> > instatiation is complete the inode doesn't really exist and isn't
> > accessible to any other task. The assertion in the ubifs code is not
> > appropriate. I suggest that you contact the ubifs maintainers.
> > Please CC me, and I will answer any questions they may have. Thank you
> > for the report."
>
> Yeah, this is the same case as with encrypted files. The assert makes
> no sense here. Thanks for reporting!
>
> Does your patch from 2017 still apply and work?
My patch is applied, but can not resolve the problem here.
The root cause is that the inode isn't locked during inode instantiation
(from Mr.Schaufler), but we check whether inode is locked in
xattr_set.
Thanks,
Xiaolei
>
> Thanks,
> //richard
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
2018-07-03 6:05 ` xiaolei li
@ 2018-07-03 7:22 ` Richard Weinberger
2018-07-03 15:22 ` Casey Schaufler
0 siblings, 1 reply; 6+ messages in thread
From: Richard Weinberger @ 2018-07-03 7:22 UTC (permalink / raw)
To: xiaolei li; +Cc: leqiao.peng, casey, linux-mtd
Am Dienstag, 3. Juli 2018, 08:05:47 CEST schrieb xiaolei li:
> Hi, Richard,
>
> On Sun, 2018-07-01 at 10:23 +0200, Richard Weinberger wrote:
> > Xiaolei Li,
> >
> > Am Donnerstag, 21. Juni 2018, 11:04:36 CEST schrieb xiaolei li:
> > > Hi Richard,
> > >
> > > I ever committed one patch[1] to fix ubifs assert in ubifs_xattr_set()
> > > if Smack is enabled.
> >
> > Ohh, that patch was missed. Sorry for that.
> >
> > > It almost always runs OK except "creating new sub-dir under a transmute
> > > enabled dir" which is found by Mr.Leqiao Peng. For this special case,
> > > there still has ubifs assert problem.
> > >
> > > With patch[1], this problem can be reproduced by these steps ( assume
> > > that the Smack has been enabled and the Smack userspace tool also
> > > installed ):
> > > 1). Add new smack rule in which the "aaa" for process object, "bbb" for
> > > file or directory object. "xwt" for the permission "execute, write and
> > > transmute"
> > >
> > > echo "aaa bbb xwt" | smackload
> > >
> > > 2). Apply the label "aaa" to current sh process. Verify the effect by
> > > "ps -Z"
> > >
> > > echo aaa > /proc/self/attr/current
> > >
> > > 3) Create a directory "test_dir" and assign the "bbb" label and enable
> > > the transmute feature upon the new directory.
> > > Verify this by "chsmack test_dir".
> > >
> > > mkdir test_dir
> > > chsmack -a "bbb" -t test_dir
> > >
> > > 4) Create a new directory "newDir" under the "test_dir".
> > >
> > > mkdir test_dir/newDir
> > >
> > >
> > > Then, ubifs assert happens, and the stack trace is:
> > >
> > > UBIFS assert failed in __ubifs_setxattr at 284 (pid 4041)
> > > CPU: 2 PID: 4041 Comm: mkdir Tainted: G S W 4.9.90 #1
> > > Call trace:
> > > [<ffffff8008089ff8>] dump_backtrace+0x0/0x1d8
> > > [<ffffff800808a254>] show_stack+0x24/0x30
> > > [<ffffff800845f074>] dump_stack+0x94/0xb8
> > > [<ffffff80083cdd78>] __ubifs_setxattr+0x358/0x5f8
> > > [<ffffff80083ce384>] ubifs_xattr_set+0x64/0x3d0
> > > [<ffffff800826c3bc>] __vfs_setxattr+0x7c/0x98
> > > [<ffffff8008416d64>] smack_d_instantiate+0x304/0x350
> > > [<ffffff800840cdec>] security_d_instantiate+0x4c/0x78
> > > [<ffffff800825ca1c>] d_instantiate+0x3c/0x70
> > > [<ffffff80083ab158>] ubifs_mkdir+0x1d8/0x1e0
> > > [<ffffff800824f140>] vfs_mkdir2+0x128/0x1b0
> > > [<ffffff8008254738>] SyS_mkdirat+0x80/0xf0
> > > [<ffffff800808378c>] __sys_trace_return+0x0/0x4
> > >
> > > Mr. Schaufler (original developer of SMACK) has analyzed the possible
> > > root cause:
> > > "I have confirmed this behavior. The issue is that the inode isn't
> > > locked during inode instantiation. It doesn't need to be, because until
> > > instatiation is complete the inode doesn't really exist and isn't
> > > accessible to any other task. The assertion in the ubifs code is not
> > > appropriate. I suggest that you contact the ubifs maintainers.
> > > Please CC me, and I will answer any questions they may have. Thank you
> > > for the report."
> >
> > Yeah, this is the same case as with encrypted files. The assert makes
> > no sense here. Thanks for reporting!
> >
> > Does your patch from 2017 still apply and work?
>
> My patch is applied, but can not resolve the problem here.
>
> The root cause is that the inode isn't locked during inode instantiation
> (from Mr.Schaufler), but we check whether inode is locked in
> xattr_set.
Care to send a massaged patch for this?
Thanks,
//richard
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
2018-07-03 7:22 ` Richard Weinberger
@ 2018-07-03 15:22 ` Casey Schaufler
2018-07-03 15:25 ` Richard Weinberger
0 siblings, 1 reply; 6+ messages in thread
From: Casey Schaufler @ 2018-07-03 15:22 UTC (permalink / raw)
To: Richard Weinberger, xiaolei li; +Cc: leqiao.peng, linux-mtd
On 7/3/2018 12:22 AM, Richard Weinberger wrote:
> Am Dienstag, 3. Juli 2018, 08:05:47 CEST schrieb xiaolei li:
>> Hi, Richard,
>>
>> On Sun, 2018-07-01 at 10:23 +0200, Richard Weinberger wrote:
>>> Xiaolei Li,
>>>
>>> Am Donnerstag, 21. Juni 2018, 11:04:36 CEST schrieb xiaolei li:
>>>> Hi Richard,
>>>>
>>>> I ever committed one patch[1] to fix ubifs assert in ubifs_xattr_set()
>>>> if Smack is enabled.
>>> Ohh, that patch was missed. Sorry for that.
>>>
>>>> It almost always runs OK except "creating new sub-dir under a transmute
>>>> enabled dir" which is found by Mr.Leqiao Peng. For this special case,
>>>> there still has ubifs assert problem.
>>>>
>>>> With patch[1], this problem can be reproduced by these steps ( assume
>>>> that the Smack has been enabled and the Smack userspace tool also
>>>> installed ):
>>>> 1). Add new smack rule in which the "aaa" for process object, "bbb" for
>>>> file or directory object. "xwt" for the permission "execute, write and
>>>> transmute"
>>>>
>>>> echo "aaa bbb xwt" | smackload
>>>>
>>>> 2). Apply the label "aaa" to current sh process. Verify the effect by
>>>> "ps -Z"
>>>>
>>>> echo aaa > /proc/self/attr/current
>>>>
>>>> 3) Create a directory "test_dir" and assign the "bbb" label and enable
>>>> the transmute feature upon the new directory.
>>>> Verify this by "chsmack test_dir".
>>>>
>>>> mkdir test_dir
>>>> chsmack -a "bbb" -t test_dir
>>>>
>>>> 4) Create a new directory "newDir" under the "test_dir".
>>>>
>>>> mkdir test_dir/newDir
>>>>
>>>>
>>>> Then, ubifs assert happens, and the stack trace is:
>>>>
>>>> UBIFS assert failed in __ubifs_setxattr at 284 (pid 4041)
>>>> CPU: 2 PID: 4041 Comm: mkdir Tainted: G S W 4.9.90 #1
>>>> Call trace:
>>>> [<ffffff8008089ff8>] dump_backtrace+0x0/0x1d8
>>>> [<ffffff800808a254>] show_stack+0x24/0x30
>>>> [<ffffff800845f074>] dump_stack+0x94/0xb8
>>>> [<ffffff80083cdd78>] __ubifs_setxattr+0x358/0x5f8
>>>> [<ffffff80083ce384>] ubifs_xattr_set+0x64/0x3d0
>>>> [<ffffff800826c3bc>] __vfs_setxattr+0x7c/0x98
>>>> [<ffffff8008416d64>] smack_d_instantiate+0x304/0x350
>>>> [<ffffff800840cdec>] security_d_instantiate+0x4c/0x78
>>>> [<ffffff800825ca1c>] d_instantiate+0x3c/0x70
>>>> [<ffffff80083ab158>] ubifs_mkdir+0x1d8/0x1e0
>>>> [<ffffff800824f140>] vfs_mkdir2+0x128/0x1b0
>>>> [<ffffff8008254738>] SyS_mkdirat+0x80/0xf0
>>>> [<ffffff800808378c>] __sys_trace_return+0x0/0x4
>>>>
>>>> Mr. Schaufler (original developer of SMACK) has analyzed the possible
>>>> root cause:
>>>> "I have confirmed this behavior. The issue is that the inode isn't
>>>> locked during inode instantiation. It doesn't need to be, because until
>>>> instatiation is complete the inode doesn't really exist and isn't
>>>> accessible to any other task. The assertion in the ubifs code is not
>>>> appropriate. I suggest that you contact the ubifs maintainers.
>>>> Please CC me, and I will answer any questions they may have. Thank you
>>>> for the report."
>>> Yeah, this is the same case as with encrypted files. The assert makes
>>> no sense here. Thanks for reporting!
>>>
>>> Does your patch from 2017 still apply and work?
>> My patch is applied, but can not resolve the problem here.
>>
>> The root cause is that the inode isn't locked during inode instantiation
>> (from Mr.Schaufler), but we check whether inode is locked in
>> xattr_set.
There is no need to do this check as all that
gets done with the decision is to print the warning.
The problem is the assumption that the inode is locked,
which is doesn't have to be during instantiation.
> Care to send a massaged patch for this?
>
> Thanks,
> //richard
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled
2018-07-03 15:22 ` Casey Schaufler
@ 2018-07-03 15:25 ` Richard Weinberger
0 siblings, 0 replies; 6+ messages in thread
From: Richard Weinberger @ 2018-07-03 15:25 UTC (permalink / raw)
To: Casey Schaufler; +Cc: xiaolei li, leqiao.peng, linux-mtd
Am Dienstag, 3. Juli 2018, 17:22:30 CEST schrieb Casey Schaufler:
> >> The root cause is that the inode isn't locked during inode instantiation
> >> (from Mr.Schaufler), but we check whether inode is locked in
> >> xattr_set.
>
> There is no need to do this check as all that
> gets done with the decision is to print the warning.
> The problem is the assumption that the inode is locked,
> which is doesn't have to be during instantiation.
I agree. Historically ubifs has a lot of (very good) asserts.
Some of them turn out to be too paranoid, like this one.
Thanks,
//richard
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-03 15:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 9:04 UBIFS assert when create new sub-dir under a transmute enabled dir with Smack enabled xiaolei li
2018-07-01 8:23 ` Richard Weinberger
2018-07-03 6:05 ` xiaolei li
2018-07-03 7:22 ` Richard Weinberger
2018-07-03 15:22 ` Casey Schaufler
2018-07-03 15:25 ` Richard Weinberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.