[LU-1518] Missing/bad operations in mdd_{obf,dot_lustre}_obj_op causing LBUGs

• Type: Bug
• Resolution: Fixed
• Priority: Blocker
• Fix Version/s: Lustre 2.3.0, Lustre 2.4.0, Lustre 2.1.4
• Affects Version/s: Lustre 2.3.0, Lustre 2.1.2, Lustre 2.1.3
• Labels: fid
• Severity: 3
• Assignee: Niu Yawei (Inactive)
• Reporter: John Hammond

Description

An unprivileged user can cause an MDS LBUG by issuing "chmod +x /mnt/lustre/.lustre/fid". Similarly root can cause an LBUG by issuing

{get,set}

facl calls against this directory. Privileged users cannot changes the attributes of /mnt/lustre/.lustre.

Issue Links

• is related to: LU-1777 open-by-fid: deadlock in lock_rename()
• is related to: LU-2780 Use real inode for .lustre/fid
• is related to: LU-1549 open-unlinked files are accessible through the PENDING directory
• is related to: LU-1550 open-by-fid getdents may return inconsistend data
• is related to: LU-1553 inaccessible files in .lustre
• is related to: LU-1552 opening lost+found: triggers (mdt_open.c:784:mdt_finish_open()) ASSERTION( ma->ma_valid & MA_INODE ) failed:
• is related to: LU-1532 only allow FID_SEQ_NORMAL from clients
• is related to: LU-1531 Accessing .lustre/fid/[0x1:0x0:0x0] triggers LBUG in osd_compat_objid_lookup()

Comments

[John Hammond]

Thanks Bob.

I pushed the commands I used in bug-notes to https://github.com/jhammond/sys. There all pretty simple and you can probably get by without them. But they are useful in that they do what it says on the can, whereas mv, ln, or ls do a lot of stating end up doing different things depending on the types of their operands, whether you added a trailing slash, and so on.

[Bob Glossman (Inactive)]

additional revisions fixed LBUGs seen in sys_rename XXX .lustre/fid and sys_rename .lustre/fid XXX. still working on other issues in bug-notes attachment.

[John Hammond]

sys_rename is an imaginary command that only calls rename(argv[1], argv[2]).

[root]# cd /mnt/lustre/
[root]# mkdir XXX
[root]# man mv
[root]# mv --no-target-directory XXX .lustre/fid

[Bob Glossman (Inactive)]

sys_mkdir? sys_rename? what are these?

I can't reproduce this with ordinary cmds:

[root@centos26 ~]# cd /mnt/lustre
[root@centos26 lustre]# mkdir XXX
[root@centos26 lustre]# rename XXX .lustre/fid
[root@centos26 lustre]#

No LBUG, no pointer dereference seen.

[John Hammond]

Results of some poking after Bob's lu1518.setattr.patch from Aug 21 2012.

[John Hammond]

With your latest patch I found one more LBUG related to the handling of .lustre/fid. If root does

cd /mnt/lustre
sys_mkdir XXX
sys_rename XXX .lustre/fid

Then you get to the same null pointer dereference osd_xattr_get(). But I didn't have much time to poke at it, so I bet there are probably more there.

[John Hammond]

Please add LU-1777 as a sub-issue, as I guess I don't have sufficient Jira clout to do so.

[Bob Glossman (Inactive)]

patch with incremental special case test in mdt_reint_setattr()

[Bob Glossman (Inactive)]

I see that cut/paste didn't go in very nicely. Will add the patch as an attachment with better formatting.

[Bob Glossman (Inactive)]

John,
I think the small additional patch below will avoid the BUG you see on chown of .lustre/fid. I really hate adding even more special case code, but I don't see a way around it in the current framework. I note that many other mdt_reint ops already have special case tests for mdt_object_obf(). I'm not expert enough to spot additional places that might also need special case checks.

--- a/lustre/mdt/mdt_reint.c
+++ b/lustre/mdt/mdt_reint.c
@@ -492,6 +492,9 @@ static int mdt_reint_setattr(struct mdt_thread_info *info,
         if (IS_ERR(mo))
                 GOTO(out, rc = PTR_ERR(mo));
 
+       if (mdt_object_obf(mo))
+               GOTO(out_put, rc = -EPERM);
+
         /* start a log jounal handle if needed */
         if (!(mdt_conn_flags(info) & OBD_CONNECT_SOM)) {
                 if ((ma->ma_attr.la_valid & LA_SIZE) ||

[Bob Glossman (Inactive)]

John, given this LBUG I'm surprised you gave http://review.whamcloud.com/#change,3726 a +review.