On Thu, May 26, 2016 at 05:04:01PM -0700, Mark Fasheh wrote: > On Fri, May 20, 2016 at 05:45:12AM +0200, Adam Borowski wrote: > > (Only btrfs currently implements dedupe_file_range.) > > > > Instead of checking the mode of the file descriptor, let's check whether > > it could have been opened rw. This allows fixing failures when deduping > > a live system: anyone trying to exec a file currently being deduped gets > > ETXTBSY. > > > > Issuing this ioctl on a ro file was already allowed for root/cap. > > Hi Adam, this patch seems reasonable to me but I have to admit to being > worried about 'unintended consequences'. I poked around the code in fs/ for > a bit and saw mostly checks against file open mode. I can't think of any unintended consequences: * root already could dedupe a file opened ro, so the code can handle that * a file being open ro but you having rw rights means you could have opened it rw There are details related to inode_permission() I admit I don't fully understand but I believe those don't really matter as reasons for not just allowing FILE_EXTENT_SAME for anyone who can read the file are quite far-fetched. > It might be that dedupe is a special case due to the potential for longer > running operations, but theoretically you'd see the same problem if trying > to exec against a file being cloned too, correct? If that's the case then > I wonder how this issue gets solved for other ioctls. Clone is a destructive operation that overwrites the file. FILE_EXTENT_SAME on the other hand makes no changes to the Posix view of the file, just to its internal representation. Meow! -- An imaginary friend squared is a real enemy. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html