From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EE56C433ED for ; Wed, 5 May 2021 13:28:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4F3BC6121F for ; Wed, 5 May 2021 13:28:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233594AbhEEN3Q (ORCPT ); Wed, 5 May 2021 09:29:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52500 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233593AbhEEN3P (ORCPT ); Wed, 5 May 2021 09:29:15 -0400 Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1696DC061574 for ; Wed, 5 May 2021 06:28:19 -0700 (PDT) Received: by mail-lj1-x233.google.com with SMTP id p12so2475403ljg.1 for ; Wed, 05 May 2021 06:28:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iHBAUjGaHBSzjioQWf1uJ+K7bCER4ubaH7dJG71fpvw=; b=BHKL8orPcvlumzz+/bfCdLUFTNfOo2GZHNYyMbBj5ESp5TlF3Y3xT8wtW+2sgGWn5S m20k17rYDwsIrQUgfF9H9ROBjtOiyteuPOd7bIx2fw3jytbR7ehsMcJ0QO6DQILeCi5o lk2i1CrbEVLfLrqO9Day1iO0Byg/ffsxWCY9JIOxrxIpHX5mXlkNiu2q8KQD1rG1sg6b o0BKQEZGB9e2VOjs0iw97QrtUZfzsZz0xQ8UWZIlAQaEcZzHTeGeIfdZ4u+FaGMnvFVo SBd9zULeOu39CGhwJzKIo5BMil2vI+pEAZVRa0Sq15NFvSFVKR7Vr27e/Pp8/BXeQ1IU 1+0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iHBAUjGaHBSzjioQWf1uJ+K7bCER4ubaH7dJG71fpvw=; b=e+h/rMnhHFMNwHYmSO/JbiJ6v5EPXAnGAp6nX7MQVa2pTUg2ubvomYxM8ED5X6Dd3n F+bL+lIXYwfRo12oA2UXtG52oSOxZwHUSxEORSaAvklh0cytrUMRAQ9AfD5v5wiQjQD4 CcYlyCB994NVYssG8eCXFZok6tFnktV3P/mWxRC72eCZQc+6whKkWvvteUnF4HfQm/II lIUKlNn3CbLbrMjXsNM64PkWtOHktLhD/1jK4oribqdJ0ECQG3TiZz5PfnN0rby5vqGT bPGV0hX1RIQ/CnMfT6kkhZ8QsglS/E38iAl5/FDM7eDJZPjBcvX0T4N9jl9xUiZGPtFh QKeA== X-Gm-Message-State: AOAM531OoiIq7FMjJ11ZO4zNmCMR6YibXibtBD6pOBYCDhTodSzLCVvr HtYxcyZwBAlrsCcd8pGnFz/LezqYHI0zNbcyFQUA2GN/ X-Google-Smtp-Source: ABdhPJx+r8/xxaqUPpUsYfC6BQcflVXF2L5olcwSWiMiE4Nl+Gi2pINf96P7O991VljdNYUHHqcCwKYmDmjUwF6qT5M= X-Received: by 2002:a2e:7f03:: with SMTP id a3mr21704945ljd.406.1620221297415; Wed, 05 May 2021 06:28:17 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Steve French Date: Wed, 5 May 2021 08:28:06 -0500 Message-ID: Subject: Re: Question regarding the patch "cifs: Fix the target file was deleted when rename failed." To: Jan Melcher Cc: CIFS Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org Your points are good ones - and it is worth revisiting the 'silly-rename' approach to a workaround (and others if people have ideas here). Although this is solved with the SMB3.1.1 POSIX Extensions (more exact semantics for the rename and delete of open files) the majority of servers don't support that yet so this is important to follow up more. Would be interested in opinion of others on these patches - and it might help for your example if you took your example above and put it in a simple script (might save a few minutes of time for others wanting to experiment with this scenario). On Wed, May 5, 2021 at 6:10 AM Jan Melcher wrote: > > I hope this is the right place for me to start a discussion regarding > a problem in the cifs file system. > > I'm experiencing the problem the patch "cifs: Fix the target file was > deleted when rename failed." > (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=9ffad9263b467efd8f8dc7ae1941a0a655a2bab2) > was trying to solve. It was further described in the samba-technical > mailing list (https://lists.samba.org/archive/samba-technical/2020-July/135592.html). > The patch was eventually reverted > (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=0e6705182d4e1b77248a93470d6d7b3013d59b30). > > Before I found the patch and the mailing list entry, I produced the > problem with the following sequence: > > $ exec 20>file # open file and leave file descriptor open > $ touch file.lock > $ mv file.lock file > mv: cannot move 'file.lock' to 'file': Permission denied > $ ls -la > total 16 > drwxr-xr-x 2 2000 2000 0 May 4 12:17 . > drwxr-xr-x 2 2000 2000 16384 May 4 12:17 .. > -rwxr-xr-x 1 2000 2000 0 May 4 12:17 file.lock > > In other words, renaming a file onto an existing file that has an open > file descriptor effectively deletes that original target file. This > happens both with samba and with a Windows server. I thzink this > command sequence seems is quite common because that's the way many > applications do file locking on posix file systems. In our case, the > problem corrupted Git repositories multiple times because of packfile > indices getting deleted. > > The patch I linked would have reduced the problem from a corruption to > a mere failed operation (the unlink-then-rename strategy is the last > resort at that place; if it is skipped, the rename fails). > > Digging through the cifs history, I found this patch > (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=14121bdccc17b8c0e4368a9c0e4f82c3dd47f240) > from 2008: "cifs: make cifs_rename handle -EACCES errors". It tried to > rename the target file to a random name (a "silly rename" I guess) and > also marked it for deletion, then tried the actual rename operation. > In my understanding, this solution should solve the mentioned problem > because renames are still allowed for files that have open file > handles. (Of course with the problem that for a short time, the target > file does not exist at all, but this problem also exists today). > > The patch has been reverted shortly after > (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=8d281efb67463fe8aac8f6e10b31492fc218bf2b) > because it would cause problems with servers that do not support busy > file renames. Maybe the situation changed since 2008 and there are > less servers that do not support busy file renames (my Windows machine > supports it), or we could find a way to re-implement the patch for > servers that do support busy file renames. The logic to handle a file > handle on the source file already tries to to a busy-file-rename by > the way. > > These are just my thoughts after two days of digging into the problems > and never having seen the cifs code before, so please forgive me if > I'm just talking nonsense. But it would be great to hear what you > think about this. > > Jan -- Thanks, Steve