linux-kernel - Re: [GIT PULL] nfsd changes for 5.18

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220712114211.GA29976@fieldses.org>
Date:   Tue, 12 Jul 2022 07:42:11 -0400
From:   Bruce Fields <bfields@...ldses.org>
To:     Igor Mammedov <imammedo@...hat.com>
Cc:     Jeff Layton <jlayton@...hat.com>,
        Chuck Lever III <chuck.lever@...cle.com>,
        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>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [GIT PULL] nfsd changes for 5.18

On Tue, Jul 12, 2022 at 10:27:46AM +0200, Igor Mammedov wrote:
> On Mon, 11 Jul 2022 14:19:41 -0400
> Bruce Fields <bfields@...ldses.org> wrote:
> 
> > On Mon, Jul 11, 2022 at 06:33:04AM -0400, Jeff Layton wrote:
> > > On Sun, 2022-07-10 at 16:42 +0000, Chuck Lever III wrote:  
> > > > > 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.
> > > >   
> > > 
> > > Cephfs allows this. My thinking at the time that I implemented it was
> > > that it should be settable for backup purposes, but this was possibly a
> > > mistake. On most filesystems, the btime seems to be equivalent to inode
> > > creation time and is read-only.  
> > 
> > So supporting it as read-only seems reasonable.
> > 
> > Clearly, failing to decode the setattr attempt isn't the right way to do
> > that.  I'm not sure what exactly it should be doing--some kind of
> > permission error on any setattr containing TIME_CREATE?
> 
> erroring out on TIME_CREATE will break client that try to
> set this attribute (legitimately). That's what by chance 
> happening with current master (return error when TIME_CREATE
> is present).

Hang on, now--our current server completely fails to decode any RPC
including a SETATTR that attempts to set TIME_CREATE, which means it
isn't able to return a useful error or tell the client which attribute
was the problem.

It's not too surprising that that would cause a problem for a client.

But failures to set supported attributes are completely normal, and if
mounts are failing completely because of that, something is really very
wrong with the client.

Could you first retest with a server that's patched to at least decode
the attribute correctly?  I suspect that may be enough.  If not, then
the client in question has a more interesting problem on its hands.

--b.