Hide Forgot
Description of problem: This is a collection of several issues found while testing the new tunegfs2 command with respect to lock table and label setting functions, -L and -o locktable=X. 1. If no lock table is specified, a label can not be set # mkfs -t gfs2 -p lock_nolock -j 1 /dev/sdb1 # tunegfs2 -L A /dev/sdb1 Label too long 2. tunegfs2 will trigger a buffer overflow if length(locktable + label) = 63 when setting the locktable name # mkfs -t gfs2 -p lock_nolock -j 1 -t foo:longername -O /dev/sdb1 Device: /dev/sdb1 Blocksize: 4096 Device Size 135.67 GB (35563885 blocks) Filesystem Size: 135.67 GB (35563883 blocks) Journals: 1 Resource Groups: 543 Locking Protocol: "lock_nolock" Lock Table: "foo:longername" UUID: 7a16773f-fbae-8863-0b92-6d83ddfe5426 # tunegfs2 -L 1234567890123456789012345678901234567890123456789012345 /dev/sdb1 # tunegfs2 -o locktable=buzzezz /dev/sdb1 # gfs2_tool sb /dev/sdb1 all mh_magic = 0x01161970 mh_type = 1 mh_format = 100 sb_fs_format = 1801 sb_multihost_format = 1900 sb_bsize = 4096 sb_bsize_shift = 12 no_formal_ino = 2 no_addr = 23 no_formal_ino = 1 no_addr = 22 sb_lockproto = lock_nolock sb_locktable = buzzezz:1234567890123456789012345678901234567890123456789012345 uuid = 7a16773f-fbae-8863-0b92-6d83ddfe5426 # wc -c buzzezz:1234567890123456789012345678901234567890123456789012345 63 # tunegfs2 -l /dev/sdb1 tunegfs2 (Jun 23 2011 05:03:50) Filesystem volume name: 1234567890123456789012345678901234567890123456789012345 Filesystem UUID: 7a16773f-fbae-8863-0b92-6d83ddfe5426 Filesystem magic number: 0x1161970 Block size: 4096 Block shift: 12 Root inode: 22 Master inode: 23 Lock Protocol: lock_nolock Lock table: buzzezz # tunegfs2 -o locktable=buzzezzz /dev/sdb1 *** buffer overflow detected ***: tunegfs2 terminated ======= Backtrace: ========= /lib64/libc.so.6(__fortify_fail+0x37)[0x31c94ff527] /lib64/libc.so.6[0x31c94fd410] /lib64/libc.so.6[0x31c94fc869] /lib64/libc.so.6(_IO_default_xsputn+0xc9)[0x31c9473a59] /lib64/libc.so.6(_IO_vfprintf+0x3826)[0x31c94476d6] /lib64/libc.so.6(__vsprintf_chk+0x9d)[0x31c94fc90d] /lib64/libc.so.6(__sprintf_chk+0x7f)[0x31c94fc84f] tunegfs2[0x40157a] tunegfs2[0x400f0a] /lib64/libc.so.6(__libc_start_main+0xfd)[0x31c941ecdd] tunegfs2[0x400ac9] 3. Setting the label name such that the locktable and label hit the 63 character limit causes `gfs2_tool sb $dev table` to print beyond the end of the lock table name # tunegfs2 -L 12345678901234567890123456789012345678901234567890123456 /dev/sdb1 # tunegfs2 -l /dev/sdb1 tunegfs2 (Jun 23 2011 05:03:50) Filesystem volume name: 12345678901234567890123456789012345678901234567890123456 Filesystem UUID: 7a16773f-fbae-8863-0b92-6d83ddfe5426 Filesystem magic number: 0x1161970 Block size: 4096 Block shift: 12 Root inode: 22 Master inode: 23 Lock Protocol: lock_nolock Lock table: buzzezz # gfs2_tool sb /dev/sdb1 table current lock table name = "buzzezz:12345678901234567890123456789012345678901234567890123456°" # gfs2_tool sb /dev/sdb1 table current lock table name = "buzzezz:12345678901234567890123456789012345678901234567890123456p" Version-Release number of selected component (if applicable): gfs2-utils-3.0.12.1-5.el6.x86_64 How reproducible: Easily Steps to Reproduce: See steps in description Actual results: Expected results: Additional info:
Created attachment 514649 [details] Upstream patch to fix bug
The upstream patch applies cleanly to the RHEL6 branch of cluster.git so I've added my Reviewed-by line and pushed the commit. Setting to POST.
https://brewweb.devel.redhat.com/buildinfo?buildID=173797
Testing gfs2-utils-3.0.12.1-7.el6.x86_64.rpm with original steps to reproduce: # mkfs -t gfs2 -O -p lock_nolock -j 1 sparse-file # tunegfs2 -L A sparse-file # tunegfs2 -l sparse-file tunegfs2 (Jul 26 2011 10:11:28) Filesystem volume name: A Filesystem UUID: 8406fc34-b5dc-10da-ae8a-a939b18015bc Filesystem magic number: 0x1161970 Block size: 4096 Block shift: 12 Root inode: 22 Master inode: 23 Lock Protocol: lock_nolock Lock table: A # mkfs -t gfs2 -p lock_nolock -j 1 -t foo:longername -O sparse-file Device: sparse-file Blocksize: 4096 Device Size 10.00 GB (2621440 blocks) Filesystem Size: 10.00 GB (2621438 blocks) Journals: 1 Resource Groups: 40 Locking Protocol: "lock_nolock" Lock Table: "foo:longername" UUID: 1551398e-1876-3e8f-8687-2897e9d39e0f # tunegfs2 -o locktable=buzzezz:1234567890123456789012345678901234567890123456789012345 sparse-file # tunegfs2 -l sparse-file tunegfs2 (Jul 26 2011 10:11:28) Filesystem volume name: buzzezz:1234567890123456789012345678901234567890123456789012345 Filesystem UUID: 1551398e-1876-3e8f-8687-2897e9d39e0f Filesystem magic number: 0x1161970 Block size: 4096 Block shift: 12 Root inode: 22 Master inode: 23 Lock Protocol: lock_nolock Lock table: buzzezz:1234567890123456789012345678901234567890123456789012345 # tunegfs2 -o locktable=buzzezzz:1234567890123456789012345678901234567890123456789012345 sparse-file Lock table name too long # gfs2_tool sb sparse-file table current lock table name = "buzzezz:1234567890123456789012345678901234567890123456789012345"
Looks like the behavior changed slightly. Now it looks like Filesystem volume name and Lock Table are the same field, which agrees now with the man page. Before the Lock Table was two parts, <lock table>:<volume name>. This would be set by `tunegfs2 -o locktable=<locktable>` and `tunegfs2 -L <volume name>.` I just want to verify that this change in behavior was intended. Also, since they are now the same field, do we really need -o locktable? Should lockproto be moved to a option like -p?
As this is technically Steve's patch I think he can answer comment #7 more authoritatively than I can.
Yes, it was intended. Not only does it now agree with the man page, but also I realised that it was going to cause all kinds of issues if we tried to separate the two. The arguments were designed to match those for tune2fs so they are just synonyms and I don't think there is any problem in leaving them "as is". We could add a -p option since tune2fs doesn't have that already, but I'm not sure that we really need to add yet another method to the tool. I don't mind doing that in the future, but its not really relevant to this bz I think. The main aim here was to ensure that the lengths of the input were being properly checked, and to fix the slightly odd behaviour of the -L flag.
Technical note added. If any revisions are required, please edit the "Technical Notes" field accordingly. All revisions will be proofread by the Engineering Content Services team. New Contents: Prior to this fix, boundaries for the locktable and label fields in the GFS2 superblock were not properly checked by the tunegfs2 tool. That allowed you to cause "gfs2_tool sb" to abort abnormally (buffer overflow) or to print invalid characters by using tunegfs2 to change the locktable or label to a minimum or maximum length (63 characters). Code was added to tunefs2 to check the correct boundaries of the locktable and label fields. As a result, tunegfs2 will no longer create invalid locktables or labels, and therefore, gfs2_tool will print the superblock values properly.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHBA-2011-1516.html