Bug 1141368
| Summary: | mv on a hardlink issues unlink and causes file loss in case mv <a> <b> and mv <b> <a> race | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Shyamsundar <srangana> | ||||||
| Component: | coreutils | Assignee: | Kamil Dudka <kdudka> | ||||||
| Status: | CLOSED WONTFIX | QA Contact: | qe-baseos-daemons | ||||||
| Severity: | high | Docs Contact: | |||||||
| Priority: | unspecified | ||||||||
| Version: | 6.0 | CC: | branto, nbalacha, ovasik, redhat, rgowdapp, vbellur | ||||||
| Target Milestone: | pre-dev-freeze | Keywords: | Patch | ||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | |||||||||
| : | 1166570 (view as bug list) | Environment: | |||||||
| Last Closed: | 2016-06-24 11:18:57 UTC | Type: | Bug | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | 1166570 | ||||||||
| Bug Blocks: | 1141172, 1142650, 1157667 | ||||||||
| Attachments: |
|
||||||||
Script can reproduce it even on my machine with ext3, so I don't think some special filesystem is needed. Non-atomicity of the mv in the case of hardlinks was reported before - http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12985 was supposed to fix it - however as your script shows, fix is probably incomplete. I saw similar activity to this but without nearly as much raciness required. I was told it was due my executing the command on cygwin. In my case I tried to do a "cp -a" which tried to copy hard links, but "pre-removed" some targeted filenames that were "in the way hardlinks". Those files were the same as some of the case-different files that it was "going to link to that location", as a result, it removed the original in trying to remove a differently cased file it thought was in the way, and then had nothing to link to the destination file (as the remove had just deleted it). I reported it as a but on the coreutils mailing list, but it was closed out as a cygwin-only bug -- and unless I wanted to construct a test case on a file system like "xfs" that has an ignore case option, it would be treated as a cygwin-only bug which would be dropped through the cracks. I was told it would be addressed fixed (~2 years back), but with it being supposedly limited to cygwin, I sorta doubted it. Now it's interesting to see the same type of problem associated with 'mv'... Of course zfs, xfs and other file systems are planning case-preserving but case-ignoring filesystems that support UTF-8. That's when some of these shortcuts will really bite. As opposed. to a "safe" move.. such as my duplicate file linker uses... renames the target to a temporary name (with "to_delete") in the name, then tries to create the hardlink from the original location. Once that has succeeded, the temp file can be delete. Reported upstream -> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18499 Thread upstream recently revived, it turns out there are multiple approaches used by various implementations: 1. Simulate but be susceptible to data loss races (what we do now) 2. Ignore rename() issue and leave both files present (what FreeBSD does) 3. Fail with a "files are identical" error (what Solaris does) All these approaches seem to be allowed by POSIX 2008. Upstream preference is to have new rename flag implemented in kernel. Is there any real-world usecase where this is likely to occur nowadays or more artificial test case type scenario only? Created attachment 959611 [details]
Patch to resolve mv/install possible hardlink race
This patch by Boris Ranto from my team seems to be the version acceptable for upstream. However - I'm a bit worried about the possible incompatibility brought into the RHEL 6 by this patch. Of course, potential data loss is always bad thing, but breaking scripts of other customers as well. I tend to use this patch only for RHEL 7 (which is in earlier phases) and probably reduce the affected parts (and potential semi-intentional regressions for some customers) a bit for RHEL 6.
I'd concur that this would potentially cause more problems in RHEL6 than it solves. No customer has ever reported the issue with the extreme edge case, and the removal of the feature will definitely be more noticeable. I'll push on with requesting a new flag to renameat() so that we can revive this feature. For reference, kernel change proposal: http://marc.info/?l=linux-api&m=141658005205610&w=2 NACK for RHEL 6.8 - and likely wontfix, as mentioned in http://marc.info/?l=linux-api&m=141660846519066&w=2 renameat won't get flags, although it might be useful to introduce some flags to control this behaviour to renameat2(). As fix for RHEL 6 would mean change in behaviour, I tend to WONTFIX that bugzilla for RHEL 6. Per comment #10, moving to RHEL-7. We already fixed this in RHEL 7 - https://bugzilla.redhat.com/show_bug.cgi?id=1166570 - moving back to RHEL 6 and closing wontfix for RHEL 6. |
Created attachment 937092 [details] Script that executes racy mv on hardlinks Description of problem: Assuming a and b are files that are hard linked. Issuing an 'mv a b' ends up in sending an unlink on a. This per se is fine, but when the same races with another 'mv b a' it ends up sending unlink on a and unlink on b causing the file to be lost as both copies are unlinked. Version-Release number of selected component (if applicable): coreutils-8.4-31.el6_5.2.x86_64 (On RHEL 6.5) coreutils-8.21-21.fc20.x86_64 (on Fedora 20) How reproducible: Using gdb always, as the race an be recreated (see below) Using the attached script, 1-8 files every 2nd or 3rd run of the script. Otherwise we hit this with GlusterFS that is mounted as a FUSE FS where it can be observed frequently, if say 1000 such files are being renamed from 2 shells on the same mount point. Steps to Reproduce: 1. With to attached script (please run under a directory that is empty) 2. Using gdb as follows, (gdb output attached) (see ===> for comments) NOTE: This is on Fedora 20 [GDB instance 1, doing a mv file file1] Reading symbols from /bin/mv...Reading symbols from /usr/lib/debug/usr/bin/mv.debug...done. done. Breakpoint 1 at 0x4059a0: file src/copy.c, line 1619. Starting program: /usr/bin/mv file file1 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". Breakpoint 1, copy_internal (src_name=src_name@entry=0x7fffffffe738 "file", dst_name=0x7fffffffe73d "file1", new_dst=new_dst@entry=false, device=device@entry=0, ancestors=ancestors@entry=0x0, x=x@entry=0x7fffffffe2d0, command_line_arg=command_line_arg@entry=true, first_dir_created_per_command_line_arg=first_dir_created_per_command_line_arg@entry=0x7fffffffe1f7, copy_into_self=copy_into_self@entry=0x7fffffffe21e, rename_succeeded=rename_succeeded@entry=0x7fffffffe21f) at src/copy.c:1619 1619 { Missing separate debuginfos, use: debuginfo-install libacl-2.2.52-4.fc20.x86_64 libattr-2.4.47-3.fc20.x86_64 libselinux-2.2.1-6.fc20.x86_64 pcre-8.33-4.fc20.x86_64 xz-libs-5.1.2-9alpha.fc20.x86_64 1635 if (x->move_mode && rename_succeeded) 1636 *rename_succeeded = false; 1638 *copy_into_self = false; 1640 if (XSTAT (x, src_name, &src_sb) != 0) 1638 *copy_into_self = false; 1640 if (XSTAT (x, src_name, &src_sb) != 0) 1646 src_mode = src_sb.st_mode; 1648 if (S_ISDIR (src_mode) && !x->recursive) 1646 src_mode = src_sb.st_mode; 1648 if (S_ISDIR (src_mode) && !x->recursive) 1658 if (command_line_arg) 1661 && x->backup_type == no_backups 1662 && seen_file (x->src_info, src_name, &src_sb)) 1669 record_file (x->src_info, src_name, &src_sb); 1672 if (!new_dst) 1684 && ! (x->move_mode || x->symbolic_link || x->hard_link 1689 : lstat (dst_name, &dst_sb)) 1690 != 0) 1687 if ((use_stat 1709 if (! same_file_ok (src_name, &src_sb, dst_name, &dst_sb, 1708 have_dst_lstat = !use_stat; 1709 if (! same_file_ok (src_name, &src_sb, dst_name, &dst_sb, 1717 if (!S_ISDIR (src_mode) && x->update) 1767 if (x->move_mode) 1614 const struct cp_options *x, 1769 if (abandon_move (x, dst_name, &dst_sb) 1770 || (unlink_src && unlink (src_name) == 0)) ===> stop here and let the other instance continue till it finishes Continuing. [Inferior 1 (process 23465) exited normally] [GDB instance 2, doing a mv file1 file] (Actually this need not be run from gdb anyway) Reading symbols from /bin/mv...Reading symbols from /usr/lib/debug/usr/bin/mv.debug...done. done. Breakpoint 1 at 0x4059a0: file src/copy.c, line 1619. Starting program: /usr/bin/mv file1 file [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". Breakpoint 1, copy_internal (src_name=src_name@entry=0x7fffffffe738 "file1", dst_name=0x7fffffffe73e "file", new_dst=new_dst@entry=false, device=device@entry=0, ancestors=ancestors@entry=0x0, x=x@entry=0x7fffffffe2d0, command_line_arg=command_line_arg@entry=true, first_dir_created_per_command_line_arg=first_dir_created_per_command_line_arg@entry=0x7fffffffe1f7, copy_into_self=copy_into_self@entry=0x7fffffffe21e, rename_succeeded=rename_succeeded@entry=0x7fffffffe21f) at src/copy.c:1619 1619 { Missing separate debuginfos, use: debuginfo-install libacl-2.2.52-4.fc20.x86_64 libattr-2.4.47-3.fc20.x86_64 libselinux-2.2.1-6.fc20.x86_64 pcre-8.33-4.fc20.x86_64 xz-libs-5.1.2-9alpha.fc20.x86_64 Continuing. [Inferior 1 (process 23475) exited normally] 3. With GlusterFS, but that is not advised :-) Actual results: Both instances of the file are unlinked Expected results: One instance should survive, if a rename was sent to the FS, one of them would have failed to find the source and hence failed that rename, while the other should have suceeded. Maybe expected behaviour from a gluster point of view, is that we should recieve a rename request in this case and not an unlink as there is no way for the FS to determine this is actually a rename otherwise. Additional info: From coreutils code from Fedora 20 debug info repository (coreutils-debuginfo-8.21-21.fc20.x86_64) on reading the code, here is what I think happens, #-> calling do_move (mv.c) -> copy(copy.c) -> copy_internal(copy.c) -> same_file_ok(copy.c) In small_file_ok we decide to set unlink_src to true, based on:: x->move_mode, same_link && 1 < dst_sb_link->st_nlink && ! same_name (src_name, dst_name) Which in copy_internal decides to unlink the source file. I am not commenting on the fix as I am not aware of other corner cases, but the cause is from the decisions as mentioned above. The fix, if deemed the right thing to do, would probably need to make it to other relase branches as well, as from a Gluster perspective, any of the other releases could act as a client mounting the gluster volume.