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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 8 Oct 2022 12:05:49 -0700 From: Kees Cook <keescook@...omium.org> To: Chuck Lever III <chuck.lever@...cle.com> Cc: Jeff Layton <jlayton@...nel.org>, "Gustavo A . R . Silva" <gustavoars@...nel.org>, Linux NFS Mailing List <linux-nfs@...r.kernel.org>, Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, Tom Rix <trix@...hat.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>, "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org> Subject: Re: [PATCH] NFSD: Avoid clashing function prototypes On Sat, Oct 08, 2022 at 03:49:45PM +0000, Chuck Lever III wrote: > > On Oct 7, 2022, at 7:54 PM, Kees Cook <keescook@...omium.org> wrote: > > > > When built with Control Flow Integrity, function prototypes between > > caller and function declaration must match. These mismatches are visible > > at compile time with the new -Wcast-function-type-strict in Clang[1]. > > > > There were 97 warnings produced by NFS. For example: > > > > fs/nfsd/nfs4xdr.c:2228:17: warning: cast from '__be32 (*)(struct nfsd4_compoundargs *, struct nfsd4_access *)' (aka 'unsigned int (*)(struct nfsd4_compoundargs *, struct nfsd4_access *)') to 'nfsd4_dec' (aka 'unsigned int (*)(struct nfsd4_compoundargs *, void *)') converts to incompatible function type [-Wcast-function-type-strict] > > [OP_ACCESS] = (nfsd4_dec)nfsd4_decode_access, > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > The enc/dec callbacks were defined as passing "void *" as the second > > argument, but were being implicitly cast to a new type. Replace the > > argument with a variable the desired to perform the casting in the > > function body. There are no resulting binary differences. > > Hi Kees, thanks for the patch. I agree this internal API could be > cleaner and more type-safe. A few things I noticed: > > - Your patch does not apply to HEAD probably because it conflicts with > 3fdc54646234 ("NFSD: Reduce amount of struct nfsd4_compoundargs that > needs clearing") Ah! Thanks, I will refresh. > - A union type might be a better fit for this application, as that > would avoid casting through an anonymous pointer. NFSD has a union > type, union nfsd4_op_u, that is ideal for this. Perfect, yes. There are similar conversions that used similar. > Would it make sense to use "union nfsd4_op_u *", such as is done in > fs/nfsd/current_stateid.h, in the definition of nfsd4_dec and nfsd4_enc ? Yup; I think that'll be perfect. > With regard to timing, I would prefer to queue this change for the > v6.2 merge window through the nfsd tree, if that's OK with you? Yeah, for sure. No rush. :) -- Kees Cook
Powered by blists - more mailing lists