[LU-17144] setxattr(XATTR_NAME_DEFAULT_LMV) should not be permitted Created: 26/Sep/23 Updated: 13/Dec/23 Resolved: 13/Dec/23 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Lustre 2.16.0 |
| Type: | Bug | Priority: | Minor |
| Reporter: | Lai Siyao | Assignee: | Lai Siyao |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Issue Links: |
|
||||
| Severity: | 3 | ||||
| Rank (Obsolete): | 9223372036854775807 | ||||
| Description |
|
Traditionally the default LOV and LMV of a directory is set via MDS_SETATTR request, but user may issue MDS_SETXATTR request with XATTR_NAME_DEFAULT_LMV to set default LMV, and this code path lack check of the default from client side, and may cause crash. The latter code path should not be allowed (like what has been done for default LOV). kernel: Lustre: gecko-OST0065: deleting orphan objects from 0x0:302777 to 0x0:302785
kernel: LustreError: 20437:0:(mdt_handler.c:4762:mdt_intent_open()) @@@ Replay open failed with -17
req@000000009fc9b77e x1777872821158784/t0(38654705669)
o101->f52141ad-20e9-4215-9f43-57ce2e03ccaf@192.168.10.62@o2ib25:668/0
lens 584/608 e 0 to 0 dl 1695680083 ref 1 fl Interpret:/4/0 rc 0/0 job:''
kernel: LustreError: 20867:0:(lod_object.c:2487:lod_dir_declare_xattr_set()) ASSERTION( buf != ((void *)0) && buf->lb_buf != ((void *)0) ) failed:
kernel: LustreError: 20867:0:(lod_object.c:2487:lod_dir_declare_xattr_set()) LBUG
kernel: Pid: 20867, comm: mdt01_003 4.18.0-425.13.1.el8_lustre.ddn17.x86_64 #1 SMP Sat Apr 15 00:59:01 UTC 2023
kernel: Lustre: gecko-OST0065: deleting orphan objects from 0x700000bd1:67843 to 0x700000bd1:67874
kernel: Lustre: gecko-OST0064: deleting orphan objects from 0x780000bd1:67740 to 0x780000bd1:67873
kernel: Lustre: gecko-OST0067: deleting orphan objects from 0x440000bd1:67748 to 0x440000bd1:67777
kernel: Lustre: gecko-OST0066: deleting orphan objects from 0x4c0000bd1:67738 to 0x4c0000bd1:67777
kernel: Call Trace TBD:
kernel: [<0>] libcfs_call_trace+0x6f/0xa0 [libcfs]
kernel: [<0>] lbug_with_loc+0x3f/0x70 [libcfs]
kernel: [<0>] lod_dir_declare_xattr_set+0x37b/0x450 [lod]
kernel: [<0>] lod_declare_xattr_set+0x804/0x1170 [lod]
kernel: [<0>] mdo_declare_xattr_set+0x79/0x380 [mdd]
kernel: [<0>] mdd_declare_xattr_set+0x2f/0x70 [mdd]
kernel: [<0>] mdd_xattr_set+0x489/0xeb0 [mdd]
kernel: [<0>] mdt_reint_setxattr+0x622/0x1290 [mdt]
kernel: [<0>] mdt_reint_rec+0x127/0x260 [mdt]
kernel: [<0>] mdt_reint_internal+0x4ac/0x7a0 [mdt]
kernel: [<0>] mdt_reint+0x5e/0x100 [mdt]
kernel: [<0>] tgt_request_handle+0xc94/0x1970 [ptlrpc]
kernel: [<0>] ptlrpc_server_handle_request+0x323/0xbd0 [ptlrpc]
kernel: [<0>] ptlrpc_main+0xbaf/0x14a0 [ptlrpc]
kernel: [<0>] kthread+0x10b/0x130
|
| Comments |
| Comment by Andreas Dilger [ 26/Sep/23 ] |
|
Would it make sense to add the check for the xattr content to this code path, rather than blocking the setxattr? This would be more flexible for backup/restore tools than having to implement a specific ioctl. I suspect in the vast majority of xattrs set are restored from a backup already, so it may be a problem to deny this. |
| Comment by Gerrit Updater [ 26/Sep/23 ] |
|
"Lai Siyao <lai.siyao@whamcloud.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52510 |
| Comment by Lai Siyao [ 26/Sep/23 ] |
|
That can be done in a separate patch, since there are quite a number of xattrs that are skipped, and should backup utils change as well? |
| Comment by Andreas Dilger [ 26/Sep/23 ] |
|
The xattrs that are skipped are ones that directly affect filesystem correctness, and do not make sense to restore (e.g. "lma"). The client will allow ll_xattr_set("trusted.lov") to be called on a file at create time appropriately, but does not send it in an MDS_SETXATTR RPC. The client could also add a check for ll_xattr_set("trusted.dmv") and call into ll_dir_setstripe(dir, lump, true) instead of the normal setxattr path. It doesn't look like the client is doing any validation beyond the LMV_USER_MAGIC even in the "good" callpath via ioctl(LL_IOC_LMV_SET_DEFAULT_STRIPE), so hopefully the MDS is checking this itself somewhere? |
| Comment by Lai Siyao [ 26/Sep/23 ] |
|
mdt_setattr_unpack() and mdt_reint_setattr() check the magic, though not complete. I'm wondering whether we need to restore the default LMV, which may not apply any more? |
| Comment by Andreas Dilger [ 26/Sep/23 ] |
|
I don't think it is for us to decide whether the trusted.dmv xattr is needed or not. In some cases it might be needed (eg. to preserve the explicit "lfs setdirstripe -D" setting on a directory tree where many files are created in a single directory). If this is not needed, then it is possible for users to exclude this from rsync via "--filter '-x trusted.lmv'". I think it does make sense to avoid explicitly saving the trusted.lmv if it matches the default value that would be used from the root directory anyway. That would avoid "hard coding" the default layout (probably "-c 1 -i -1") for all of the copied directories in the common case where the directories are only using the default value. |
| Comment by Gerrit Updater [ 13/Dec/23 ] |
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/52510/ |
| Comment by Peter Jones [ 13/Dec/23 ] |
|
Landed for 2.16 |