Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Igor Mammedov <imammedo@redhat.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Ondrej Valousek <ondrej.valousek.xm@renesas.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Bruce Fields <bfields@fieldses.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Jeff Layton <jlayton@redhat.com>
Subject: Re: [GIT PULL] nfsd changes for 5.18
Thread-Topic: [GIT PULL] nfsd changes for 5.18
Thread-Index: AQHYPS20r3Ca7pclI0+jw5IFRtIPbK14GX4AgABkTAA=
Date:   Sun, 10 Jul 2022 16:42:43 +0000
Message-ID: <B62B3A57-A8F7-478B-BBAB-785D0C2EE51C@oracle.com>
References: <EF97E1F5-B70F-4F9F-AC6D-7B48336AE3E5@oracle.com>
 <20220710124344.36dfd857@redhat.com>
In-Reply-To: <20220710124344.36dfd857@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?mnf5ApbXkAJoUWbtlZWABlLMFZ5Yw4g+eXcMnvL17yS7Gs3V4V76Q95Pq+n7?=
 =?us-ascii?Q?jktpAji3xJhezXcSRqKMpmgAZy58AULohYU7qkgm3Ft23/q/N9VPnvzMFbZv?=
 =?us-ascii?Q?aW+NTHxOD87F72lEftiab/E6dTltSc4RLfY5CfJ7GUSLTyt+UdCMy0MvHIvP?=
 =?us-ascii?Q?PFc3F5cLGdFrSUBlqdH3lm09dpPVsmU+KAxT6kXPxYjQugwHIevXCEMF8gsK?=
 =?us-ascii?Q?8OFHgh2Jbl1c9gCQ7i5sew+Ux41FFCUfRMYkEBatTGE7gONh/uN481/ahQTP?=
 =?us-ascii?Q?XFQPGqMa/15GtASTU6t8vDVgIRp/HVaIZ2VrLJHxmDVWJDtKFOgzrqRpmxwh?=
 =?us-ascii?Q?Qe0qqCwwsh81uv1rnIZ3qkeTK2ETO1D4AsfjKeOPdnMJwqZvbbuX115B08vE?=
 =?us-ascii?Q?dXR8gQ8uROJQd6fE/Hclistrj1XUlmIdd7BEs9WX3+KWBtWnj5yv9inOBqnK?=
 =?us-ascii?Q?KZJceGCj3UoX+s05lprIRhpOdzg7XtafHCoD4j8EWln3KH9MLI33ms/+yBxw?=
 =?us-ascii?Q?UvfWqZKbZcTDx9D9vhpzzayK0X1HgWxkGwmf3VYVWlXSsbxnDy2ZV/yJ5st0?=
 =?us-ascii?Q?5UKezSsliqIoE/IcYK22FU5cgnMFvZusOEgfM3iTMn7QCuIkrGDh0VSwve8n?=
 =?us-ascii?Q?0J3XMTfRa9sgeqRasz+h7zvDKY5DQt0USY8aC/QP3PuOACIy2QK9etv36Bot?=
 =?us-ascii?Q?pJDawiXrpz+Tp535URkoYSFOO6HykncDN3wQ+Q6OU2E9RzZk6kUG9YSs3FAW?=
 =?us-ascii?Q?GK8e2L3ImACdeUXCjeUcVgjZWQH11FXcDPhyvCoR14nhHOqrSS8ymUo3qgSk?=
 =?us-ascii?Q?qr1x2RKt48aGypacBrCfWrW6tMsQUZyUUlKz7nfVbrzxC5QWT+aYAMwHDjTw?=
 =?us-ascii?Q?ab3Uy23h0YyhVPr5KHv1fwMCFAXuhh8pq77hTEly2zyZSvP1uoATlPVWGlXw?=
 =?us-ascii?Q?VgFOq/HrqroxRzGo/1RQPraKvQS8M+MwhX/U6kIalkG1Y/ge85furoAqAhDi?=
 =?us-ascii?Q?o8g7u683YyKppjVQWliZgOg7UmYTyCkS0C5uA2/tOYAwcnvOAqpOR14kZ0fe?=
 =?us-ascii?Q?mpF5LCf7hFWk/8uYBO1F7bkQ30eTtdYQruh5wWe3Aw2qJICxGFGBGfhj387Y?=
 =?us-ascii?Q?8YYEce0Yme/YzNoCx09aBMHZG/8hZSX81kCuTQQ99e7Hw0BF5cDyTeuo47ym?=
 =?us-ascii?Q?1OdSGPYOpLXiIKuVsTBjAG4eOFS9fOEk4pqKoXCw9bWzg9DLuV4OrmfjYXA2?=
 =?us-ascii?Q?6d+MTKJesbK+lILIY7Hou5Y8o3K8ZkHHLCoJgjHrsSS15/rV3LdOtY2xv6/f?=
 =?us-ascii?Q?TlhRKZaK22SUHsua7usqgM8+oIx8KizqQW4psp20pd1CCiV6X03UsOMaU3Iu?=
 =?us-ascii?Q?hnIUgUIi0CYvSV3eLQqShTy9uC/blsvcHCLcrG9c1wRIY+9WT4BM1g9TO2CU?=
 =?us-ascii?Q?WpNxyCV/qYKcyIsQGiszQ8ElJBazcGvXWxY9jsdJwdtKzFC7qBSVEKEirDFo?=
 =?us-ascii?Q?TuVgFxPMwKBgbP+KLJ5Od6oCmj40h7TsRL8X7HwmvUg93SFyF/dtHeoag2nj?=
 =?us-ascii?Q?LCRbKCN6jO8GKXAxhXTB/8wJ9RHUwCZadSHPwAlYDA2LD42fceryseX69K/V?=
 =?us-ascii?Q?rw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <AF180342595857469E0BD28B908BE491@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 55a3dee6-e604-4003-d7a4-08da629335f2
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jul 2022 16:42:43.1012
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: IaQnTijElXMI6DmJqXQmyOIwNNw2VYOHsDSOptyWN1aQ6uupCRib+xR91glUQYMtKSi4x9+l/qrXbvBETKmxnw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR10MB5416
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-10_16:2022-07-08,2022-07-10 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999 mlxscore=0
 suspectscore=0 phishscore=0 spamscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207100074
X-Proofpoint-GUID: tzDWZOIb6YzLJxklu8UnN9qCFN_d954_
X-Proofpoint-ORIG-GUID: tzDWZOIb6YzLJxklu8UnN9qCFN_d954_
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jul 10, 2022, at 6:43 AM, Igor Mammedov <imammedo@redhat.com> wrote:
>=20
> On Mon, 21 Mar 2022 14:12:31 +0000
> Chuck Lever III <chuck.lever@oracle.com> wrote:
>=20
> couldn't find offender patch on ML so replying here

Probably:

https://lore.kernel.org/linux-nfs/AEC24099-5BC9-49C8-B759-920824F23F3C@orac=
le.com/


>> Hi Linus-
>>=20
>> The following changes since commit 7e57714cd0ad2d5bb90e50b5096a0e671dec1=
ef3:
>>=20
>> Linux 5.17-rc6 (2022-02-27 14:36:33 -0800)
>>=20
>> are available in the Git repository at:
>>=20
>> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git tags/nfsd-5.=
18
>>=20
>> for you to fetch changes up to 4fc5f5346592cdc91689455d83885b0af65d71b8:
>>=20
>> nfsd: fix using the correct variable for sizeof() (2022-03-20 12:49:38 -=
0400)
>>=20
>> ----------------------------------------------------------------
>> New features:
>> - NFSv3 support in NFSD is now always built
>> - Added NFSD support for the NFSv4 birth-time file attribute
> [...]
>=20
>> Ondrej Valousek (1):
>> nfsd: Add support for the birth time attribute

Thank you for the report, Igor.


> This patch regressed clients that support TIME_CREATE attribute.
> Starting with this patch client might think that server supports
> TIME_CREATE and start sending this attribute in its requests.

Indeed, e377a3e698fb ("nfsd: Add support for the birth time
attribute") does not include a change to nfsd4_decode_fattr4()
that decodes the birth time attribute.

I don't immediately see another storage protocol stack in our
kernel that supports a client setting the birth time, so NFSD
might have to ignore the client-provided value.


> However kernel on server side (since this patch and to
> current master) upon getting such request will return EINVAL.
> (my guess is that TIME_CREATE not being decoded properly and
> that messes up request parsing).

I'll send a quick-and-dirty fix your way as we explore the
question of whether NFSD needs to ignore the birth time value
in this case.


> End result is unusable mount (unless it's treated as readonly).

That seems odd, and not clear whether that's a client or server
problem. I hope that will clear up once the server deals with
the time_create attribute appropriately.


> Reproduces with current master (HEAD at e5524c2a1fc40) and MacOS
> client (Big Sur or newest Monterey).
>=20
> server is typical setup exporting files from XFS (Fedora36)
>=20
> #  rpcdebug -m nfsd -s all
>=20
> on client:
>=20
> % mount -t nfs -o vers=3D4,rw,nfc,sec=3Dsys testnas:/mnt  ~/test
> % touch ~/test/fff
>     touch: test/fff: Invalid argument
>=20
> server logs:
>=20
> nfsd: fh_compose(exp fd:00/128 fff, ino=3D0)
> NFSD: nfsd4_open filename  op_openowner 0000000000000000
>=20
> Here is a request the touch generates:
>        Network File System, Ops(6): PUTFH, SAVEFH, OPEN, GETATTR, RESTORE=
FH, GETATTR
>            [Program Version: 4]
>            [V4 Procedure: COMPOUND (1)]
>            Tag: create
>            minorversion: 0
>            Operations (count: 6): PUTFH, SAVEFH, OPEN, GETATTR, RESTOREFH=
, GETATTR
>                Opcode: PUTFH (22)
>                Opcode: SAVEFH (32)
>                Opcode: OPEN (18)
>                    seqid: 0x00000004
>                    share_access: OPEN4_SHARE_ACCESS_BOTH (3)
>                    share_deny: OPEN4_SHARE_DENY_NONE (0)
>                    clientid: 0xba93c9620aec46ea
>                    owner: <DATA>
>                    Open Type: OPEN4_CREATE (1)
>                        Create Mode: UNCHECKED4 (0)
>                        Attr mask: 0x00040002 (Mode, Time_Create)
>                            reco_attr: Mode (33)
>                            reco_attr: Time_Create (50)
>                    Claim Type: CLAIM_NULL (0)
>                        Name: fff
>=20
>        [...]
>=20
> when trying to copy file via GUI (Finder) it goes a different route
> but ends up with error anyway and with leftover 0-length file on server
> with messed up permissions, i.e.

The current NFSv4 OPEN(CREATE) code path is still not right. Fixing
the TIME_CREATE problem should make this symptom go away for now,
but eventually that path will need to be restructured so that it
cannot leave a turd if the whole create process was not able to
complete.


> open/create without Time_Create succeeds but followup
> setattr with Time_Create fails EINVAL.
>=20
>        Network File System, Ops(3): PUTFH, SETATTR, GETATTR
>            [Program Version: 4]
>            [V4 Procedure: COMPOUND (1)]
>            Tag: setattr
>            minorversion: 0
>            Operations (count: 3): PUTFH, SETATTR, GETATTR
>                Opcode: PUTFH (22)
>                Opcode: SETATTR (34)
>                    StateID
>                    Attr mask: 0x00450002 (Mode, Time_Access_Set, Time_Cre=
ate, Time_Modify_Set)
>                        reco_attr: Mode (33)
>                        reco_attr: Time_Access_Set (48)
>                        reco_attr: Time_Create (50)
>                        reco_attr: Time_Modify_Set (54)
>                Opcode: GETATTR (9)
>            [Main Opcode: SETATTR (34)]
>=20
> [...]
>> --
>> Chuck Lever

--
Chuck Lever