xfs_logprint can't handle multiply-logged inode fields
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| xfsprogs (Ubuntu) |
Fix Released
|
Undecided
|
Unassigned | ||
| Trusty |
Fix Released
|
Medium
|
Eric Desrochers | ||
Bug Description
[Impact]
* Under certain conditions (such as when selinux is enabled and probably other ways) xfsprogs on Trusty may report a error "illegal inode type" and SIGABRT, generating a coredump as follow :
Using host libthread_db library "/lib/x86_
Core was generated by `xfs_logprint -c /dev/mapper/
Program terminated with signal SIGABRT, Aborted.
when more than one flag is set on f->ilf_fields. (Example : When data+attr are set)
[Explanation]
As we speak, the switch() statement is doing a Binary AND between ifl_fields & XFS_ILOG_NONCORE.
switch (f->ilf_fields & XFS_ILOG_NONCORE) {
XFS_ILOG_NONCORE being the sum of a Binary OR for all the Inode changes to log:
#define XFS_ILOG_NONCORE (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
When more than 1 flag is set in "ifl_fields"
(Example took from the coredump in gdb "ilf_fields = 133")
-------
#define XFS_ILOG_DDATA 0x002 /* log i_df.if_data */
#define XFS_ILOG_DEXT 0x004 /* log i_df.if_extents */
#define XFS_ILOG_DBROOT 0x008 /* log i_df.i_broot */
#define XFS_ILOG_DEV 0x010 /* log the dev field */
#define XFS_ILOG_UUID 0x020 /* log the uuid field */
#define XFS_ILOG_ADATA 0x040 /* log i_af.if_data */
#define XFS_ILOG_AEXT 0x080 /* log i_af.if_extents */
#define XFS_ILOG_ABROOT 0x100 /* log i_af.i_broot */
ifl_field = 133 & XFS_ILOG_NONCORE= 510 = 132
-------
and cannot match any case statement based on the inode flags value above and has no choice but to use default: statement because none of them are true and call xlog_panic().
[Test Case]
1) Create a XFS device (dev/vdb1)
2) apt-get dist-upgrade
3) Remove apparmor # To avoid potential conflict with selinux.
4) Reboot
5) Installed selinux # so that data+attr is set, and logprinting.
6) Reboot
7) Run xfs_logprint against /dev/vdb1
- sudo xfs_logprint -c /dev/vdb1
There is for sure other ways to trigger this behavior but that is the one I used base on the git commit log:
"I've tested this by a simple test such as creating one
file on an selinux box, so that data+attr is set, and
logprinting"
[Regression Potential]
* This is a rework of "xlog_print_
The rework offer a better detection instead of a one size fits all, and provide various switch() statement context and appropriate actions.
It is also capable to handle multiply-logged inode fields which current Trusty version can't handle.
The uploaded package has also been tested as a test package by some affected users (pre-SRU) and everyone confirmed it was working with no more error and crashes.
[Other Info]
* The patch never land in Trusty because the package was a copy from it's predecessor release, saucy, and never been SRU since then.
* Upstream commit:
https:/
* Trusty AFFECTED ONLY.
$ git describe --contains dda4129
v3.1.11~29
$ rmadison xfsprogs
==> xfsprogs | 3.1.9ubuntu2 | trusty <==
xfsprogs | 4.3.0+nmu1ubuntu1 | xenial
xfsprogs | 4.3.0+nmu1ubuntu1.1 | xenial-updates
xfsprogs | 4.9.0+nmu1ubuntu1 | artful
xfsprogs | 4.9.0+nmu1ubuntu1 | bionic
[Orig Description]
It has been brought to my attention that the following :
"
The command 'xfs_logprint -c <DEVICE>' coredump on Trusty and display the error :
xlog_print_
"
| Eric Desrochers (slashd) wrote | #1 |
| description: | updated |
| description: | updated |
| Changed in xfsprogs (Ubuntu): | |
| status: | New → Fix Released |
| description: | updated |
| description: | updated |
| description: | updated |
| description: | updated |
| description: | updated |
| description: | updated |
| description: | updated |
| Changed in xfsprogs (Ubuntu Trusty): | |
| status: | New → In Progress |
| assignee: | nobody → Eric Desrochers (slashd) |
| description: | updated |
| description: | updated |
| Changed in xfsprogs (Ubuntu Trusty): | |
| importance: | Undecided → Medium |
| Eric Desrochers (slashd) wrote | #3 |
| description: | updated |
| description: | updated |
| Łukasz Zemczak (sil2100) wrote Please test proposed package | #5 |
| Changed in xfsprogs (Ubuntu Trusty): | |
| status: | In Progress → Fix Committed |
| tags: | added: verification-needed verification-needed-trusty |
| Eric Desrochers (slashd) wrote | #6 |
| tags: |
added: verification-done-trusty removed: verification-needed-trusty |
| tags: |
added: verification-done removed: verification-needed |
| Launchpad Janitor (janitor) wrote | #7 |
| Changed in xfsprogs (Ubuntu Trusty): | |
| status: | Fix Committed → Fix Released |
| Brian Murray (brian-murray) wrote Update Released | #8 |
I have looked at the 3 coredumps, and it is always the same type of crash. The program terminated with an abort signal (SIGABRT). In this case SIGABRT is called by libc and other libraries to abort the program due to error.
# xfsprogs: logprint/log_misc.c
---
default: {
xlog_panic(
}
}
return 0;
} /* xlog_print_
---
# gdb - post-crash analysis
---
Using host libthread_db library "/lib/x86_
Core was generated by `xfs_logprint -c /dev/mapper/
Program terminated with signal SIGABRT, Aborted.
#0 0x00007f534e0dfc37 in __GI_raise (sig=sig@entry=6) at ../nptl/
56 ../nptl/
(gdb) bt
#0 0x00007f534e0dfc37 in __GI_raise (sig=sig@entry=6) at ../nptl/
#1 0x00007f534e0e3028 in __GI_abort () at abort.c:89
#2 0x00000000004323e1 in xlog_panic (fmt=<optimized out>) at util.c:129
#3 0x000000000040447e in xlog_print_
#4 0x0000000000404cf9 in xlog_print_record (fd=<optimized out>, num_ops=68, len=<optimized out>, read_type=
#5 0x0000000000404f49 in xfs_log_print (log=log@
#6 0x0000000000401eba in main (argc=<optimized out>, argv=<optimized out>) at logprint.c:241
(gdb) frame 3
#3 0x000000000040447e in xlog_print_
748 log_misc.c: No such file or directory.
(gdb) p *f
$1 = {ilf_type = 4667, ilf_size = 4, ==>ilf_fields = 133<==, ilf_asize = 16, ilf_dsize = 16, ilf_ino = 21772003, ilf_u = {ilfu_rdev = 0, ilfu_uuid = '\000' <repeats 15 times>}, ilf_blkno = 7740272,
ilf_len = 16, ilf_boffset = 768}
---
It seems like the "ilf_fields" value is outside the range of what xfsprogs is expecting too. Forcing the code to pick the default: statement, display the error and then xlog_panic().
There is an upstream commit that does a rework of the function xlog_print_
I have strong believe this rework will fix the actual situation.