linux-kernel - Re: [PATCH 1/3] nfsd: bring in support for delstid draft XDR encoding

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: <b7d76076fe593b0c24ce983db9a0dbacc9fbfa5d.camel@kernel.org>
Date: Mon, 19 Aug 2024 16:04:26 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Chuck Lever III <chuck.lever@...cle.com>
Cc: Neil Brown <neilb@...e.de>, Dai Ngo <dai.ngo@...cle.com>, Olga
 Kornievskaia <okorniev@...hat.com>, Tom Talpey <tom@...pey.com>, Trond
 Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>, Tom
 Haynes <loghyr@...il.com>, Linux Kernel Mailing List
 <linux-kernel@...r.kernel.org>,  Linux NFS Mailing List
 <linux-nfs@...r.kernel.org>
Subject: Re: [PATCH 1/3] nfsd: bring in support for delstid draft XDR
 encoding

On Mon, 2024-08-19 at 19:50 +0000, Chuck Lever III wrote:
> 
> 
> > On Aug 19, 2024, at 9:26 AM, Jeff Layton <jlayton@...nel.org>
> > wrote:
> > 
> > I'm playing with the new version now and it seems to be much
> > improved.
> > Only two real bugs I've hit at this point:
> > 
> > 1/ Some of the struct specifications need to be typedefs as well.
> > For
> > instance, the delstid draft refers to "nfstime4", but the
> > autogenerated
> > struct definition doesn't have the typedef for it. It may be best
> > to
> > just add typedefs for all of these sorts of structs.
> 
> What's the specific symptom? I've been able to catenate nfs4_1.x
> and delstid.x, xdrgen builds the header and source without tossing
> any exceptions, and gcc compiles it without complaint.
> 


Basically, I was getting this when I'd convert nfs4_1.x to a header:

struct nfstime4 {
        int64_t seconds;
        uint32_t nseconds;
};

...but the delstid header has these:

typedef nfstime4 fattr4_time_deleg_access;

typedef nfstime4 fattr4_time_deleg_modify;


...nothing defined nfstime4 in this case.

> AFAICT, xdrgen will add "struct" where it's necessary.
> 
> I've been squirrelly about using "typedef" too often because
> the Linux kernel's coding style is to avoid C typedefs for
> shorthand structure names.
> 

Oh, ok. I didn't concatenate the files like you did and just generated
the delstid files separately from the nfs4_1 ones. I guess that throws
off the dependency tracking that you're doing here for typedefs.

> 
> > 2/ xdrgen_encode_nfstime4 want a pointer to the nfstime4, but the
> > autogenerated code for xdrgen_encode_fattr4_time_deleg_access and
> > xdrgen_encode_fattr4_time_deleg_modify try to pass it by value
> > instead.
> 
> Here's my generated copy of xdrgen_encode_fattr_time_deleg_access:
> 
> /* typedef fattr4_time_deleg_access */
> static bool
> __maybe_unused                                                     
> xdrgen_encode_fattr4_time_deleg_access(struct xdr_stream *xdr, const
> fattr4_time_deleg_access value)
> {
> 	/* (basic) */
> 	return xdrgen_encode_nfstime4(xdr, &value);
> };
> 
> Looks like it does the right thing...?

Probably another side-effect of it not knowing what to do with nfstime4
when I convert the delstid draft. Concatenating them seems unwieldy but
I guess that would work. I do like being able to keep generated code
from different files separate though.

-- 
Jeff Layton <jlayton@...nel.org>