linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: jejb@linux.ibm.com
Cc: denkenz@gmail.com, dhowells@redhat.com,
	Nathan Chancellor <natechancellor@gmail.com>,
	Eric Biggers <ebiggers@google.com>,
	zohar@linux.vnet.ibm.com, jmorris@namei.org, serge@hallyn.com,
	linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KEYS: trusted: fix -Wvarags warning
Date: Fri, 12 Oct 2018 10:14:52 -0700	[thread overview]
Message-ID: <CAKwvOdnejRka4hP66zCGS+MSXx_z335qZUTU8J0UbEq-boRYjQ@mail.gmail.com> (raw)
In-Reply-To: <1539360075.2656.18.camel@linux.ibm.com>

On Fri, Oct 12, 2018 at 9:01 AM James Bottomley <jejb@linux.ibm.com> wrote:
>
> On Fri, 2018-10-12 at 10:53 -0500, Denis Kenzior wrote:
> > Hi James,
> >
> > > >   From the links provided in the patch it seems that one cannot
> > > > pass char/float/short to va_start().  Fair enough.  So if we make
> > > > h3 an unsigned int, the issue goes away, no?
> > >
> > > For the current version of clang, yes.  However, if we're fixing
> > > this for good a char * pointer is the only guaranteed thing because
> > > it mirrors current use in printf.
> > >
> >
> > All right.  I guess I wasn't aware that non-printf like variadic
> > functions are now considered harmful or of the impending crusade
> > against them :)
>
> It's not, it's just a maintainer issue: The original problem is because
> we coded for gcc specifically; it doesn't complain and does the right
> thing, so everyone was happy.  Now Clang comes along and is unhappy
> with this, so the question a good maintainer should ask is "how do I
> fix this so it never comes back again?", not "what's the easiest
> bandaid to get both Clang and gcc to work?" because the latter is how
> we got here in the first place.

Clang is simply pointing out that this is explicitly undefined
behavior by the C90 spec.  As such, not only may the implementation be
different between compilers, but it is free to change between versions
of the same compiler (I haven't witnessed such a case yet in my
limited career, but I would never say never when it comes to relying
on undefined behavior).

>
> James
>
> > But in the context of this patch, can we please use something less
> > invasive than changing all the arguments around?  Promoting h3 to a
> > bool (if possible) or int/unsigned int would get my vote.
> >
> > Regards,
> > -Denis
> >
>


-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2018-10-13  0:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-09 22:11 undefined behavior (-Wvarargs) in security/keys/trusted.c#TSS_authhmac() Nick Desaulniers
2018-10-11 16:02 ` Arnd Bergmann
2018-10-11 16:10   ` James Bottomley
2018-10-11 20:31     ` [PATCH] KEYS: trusted: fix -Wvarags warning ndesaulniers
2018-10-12  1:50       ` Nathan Chancellor
2018-10-12 16:55         ` Nick Desaulniers
2018-10-12 17:03           ` Nathan Chancellor
2018-10-12 12:29       ` Denis Kenzior
2018-10-12 15:05         ` James Bottomley
2018-10-12 15:13           ` Denis Kenzior
2018-10-12 15:22             ` James Bottomley
2018-10-12 15:44               ` Denis Kenzior
2018-10-12 15:46                 ` James Bottomley
2018-10-12 15:53                   ` Denis Kenzior
2018-10-12 16:01                     ` James Bottomley
2018-10-12 17:14                       ` Nick Desaulniers [this message]
2018-10-12 15:25             ` James Bottomley
2018-10-12 17:05             ` Nick Desaulniers
2018-10-12 17:17               ` Nick Desaulniers
2018-10-12 17:27               ` Denis Kenzior
2018-10-12 18:39                 ` Nick Desaulniers
2018-10-12 17:02         ` Nick Desaulniers
2018-10-12 17:15           ` Denis Kenzior
2018-10-15  9:26       ` David Laight
2018-10-15  9:26         ` David Laight
2018-10-15 21:53         ` Nick Desaulniers
2018-10-16  8:13           ` David Laight
2018-10-16  8:13             ` David Laight
2018-10-22 23:43             ` [PATCH v2] " ndesaulniers
2018-10-23  0:00               ` Nathan Chancellor
2018-10-24  8:36               ` Jarkko Sakkinen
2018-10-29 17:54                 ` Nick Desaulniers
2019-02-11 18:36                   ` Nick Desaulniers
2019-02-12 23:12                     ` Jarkko Sakkinen
2019-02-14 10:52                       ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKwvOdnejRka4hP66zCGS+MSXx_z335qZUTU8J0UbEq-boRYjQ@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=denkenz@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=ebiggers@google.com \
    --cc=jejb@linux.ibm.com \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=natechancellor@gmail.com \
    --cc=serge@hallyn.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).