Message-ID: <B62B3A57-A8F7-478B-BBAB-785D0C2EE51C@oracle.com>
Date:   Sun, 10 Jul 2022 16:42:43 +0000
From:   Chuck Lever III <chuck.lever@...cle.com>
To:     Igor Mammedov <imammedo@...hat.com>
CC:     Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
        Ondrej Valousek <ondrej.valousek.xm@...esas.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Bruce Fields <bfields@...ldses.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jeff Layton <jlayton@...hat.com>
Subject: Re: [GIT PULL] nfsd changes for 5.18



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

Probably:

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


>> Hi Linus-
>> 
>> The following changes since commit 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3:
>> 
>> Linux 5.17-rc6 (2022-02-27 14:36:33 -0800)
>> 
>> are available in the Git repository at:
>> 
>> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git tags/nfsd-5.18
>> 
>> for you to fetch changes up to 4fc5f5346592cdc91689455d83885b0af65d71b8:
>> 
>> nfsd: fix using the correct variable for sizeof() (2022-03-20 12:49:38 -0400)
>> 
>> ----------------------------------------------------------------
>> New features:
>> - NFSv3 support in NFSD is now always built
>> - Added NFSD support for the NFSv4 birth-time file attribute
> [...]
> 
>> 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).
> 
> server is typical setup exporting files from XFS (Fedora36)
> 
> #  rpcdebug -m nfsd -s all
> 
> on client:
> 
> % mount -t nfs -o vers=4,rw,nfc,sec=sys testnas:/mnt  ~/test
> % touch ~/test/fff
>     touch: test/fff: Invalid argument
> 
> server logs:
> 
> nfsd: fh_compose(exp fd:00/128 fff, ino=0)
> NFSD: nfsd4_open filename  op_openowner 0000000000000000
> 
> Here is a request the touch generates:
>        Network File System, Ops(6): PUTFH, SAVEFH, OPEN, GETATTR, RESTOREFH, 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
> 
>        [...]
> 
> 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.
> 
>        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_Create, 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)]
> 
> [...]
>> --
>> Chuck Lever

--
Chuck Lever

Powered by blists - more mailing lists