* 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 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).