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 for Android: free password hash cracker in your pocket

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

Message-ID: <67E54CBA-CC53-4BA6-A62D-3F594102AE6C@oracle.com>
Date: Mon, 19 Aug 2024 13:28:35 +0000
From: Chuck Lever III <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>
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 Aug 19, 2024, at 9:26 AM, Jeff Layton <jlayton@...nel.org> wrote:
> 
> On Mon, 2024-08-19 at 13:21 +0000, Chuck Lever III wrote:
>> 
>>> On Aug 19, 2024, at 7:44 AM, Jeff Layton <jlayton@...nel.org> wrote:
>>> 
>>> On Mon, 2024-08-19 at 10:04 +1000, NeilBrown wrote:
>>>> On Fri, 16 Aug 2024, Jeff Layton wrote:
>>>> 
>>>>> +// Generated by lkxdrgen, with hand-edits.
>>>> 
>>>> I *really* don't like having code in the kernel that is partly
>>>> tool-generated and partly human-generated, and where the boundary isn't
>>>> obvious (like separate files).
>>>> 
>>>> If we cannot use tool-generated code as-is, then let's fix the tool.
>>>> If we cannot fix the tool, then include the raw output and a
>>>> human-generated patch which the makefile combines.
>>>> 
>>>> Ideally the tool should be in tools/, the .x file should be in fs/nfsd/
>>>> and the makefile should apply the one to the other.  We are going to
>>>> want to do that eventually and I think it should be priority.  The tool
>>>> doesn't have to be bug-free before it lands (nothing is).
>>>> 
>>>> A particular reason for this is that I cannot review tool-generated
>>>> hand-editted code.  It is too noisy and I don't know which parts are
>>>> worth closer inspection etc.
>>> 
>>> Fair point. Chuck made some similar comments to me privately, and it
>>> looks like he has updated his xdrgen tool as well.
>>> 
>>> I'll plan to just respin that part from scratch and regenerate from the
>>> .x files. I'll also keep my hand-edits in a separate commit for the
>>> next version.
>>> 
>>> It'll probably take me a few days, but I'll try to have something to
>>> resend within the next week or so.
>> 
>> Meanwhile, I'll post the current xdrgen tool for review. It
>> already lives under tools/ as Neil suggested above.
>> 
>> I'm hoping that by providing the .x snippet used to generate the
>> source, and by adding one or two "pragma" annotations to the tool
>> to handle certain exceptions, no hand-edits or extra patching
>> will be needed.
>> 
>> 
> 
> 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.
> 
> 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.
> 
> These are minor and easily fixed, obviously, but it would be nice to
> not need to do that.

That might be a bug in the spec, actually. I'll have a look.

--
Chuck Lever