[PATCH] proposal adding explicit_bzero

[PATCH] proposal adding explicit_bzero

[David CARLIER]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

Hi dear lists,
this is my first message so forgive me if this idea had already been rejected.

Kind regards.

[-- Attachment #2: 0001-string-adding-simple-explicit_bzero-implementation.patch --]
[-- Type: text/x-patch, Size: 1284 bytes --]

From c0a16cf96b96b009097d6ed656a2a7b8969e8399 Mon Sep 17 00:00:00 2001
From: David Carlier <dcarlier@afilias.info>
Date: Fri, 15 Jun 2018 13:30:09 +0000
Subject: [PATCH] string: adding simple explicit_bzero implementation.

glibc implementing it and modern security based code starting
using it widely, here a simple implementation using memory barrier.
---
 include/string.h            | 1 +
 src/string/explicit_bzero.c | 8 ++++++++
 2 files changed, 9 insertions(+)
 create mode 100644 src/string/explicit_bzero.c

diff --git a/include/string.h b/include/string.h
index ce1dc300..795a2abc 100644
--- a/include/string.h
+++ b/include/string.h
@@ -82,6 +82,7 @@ void *memccpy (void *__restrict, const void *__restrict, int, size_t);
 char *strsep(char **, const char *);
 size_t strlcat (char *, const char *, size_t);
 size_t strlcpy (char *, const char *, size_t);
+void explicit_bzero (void *, size_t);
 #endif
 
 #ifdef _GNU_SOURCE
diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
new file mode 100644
index 00000000..47dba3c7
--- /dev/null
+++ b/src/string/explicit_bzero.c
@@ -0,0 +1,8 @@
+#define _BSD_SOURCE
+#include <string.h>
+
+void explicit_bzero(void *d, size_t n)
+{
+	memset(d, 0, n);
+	__asm__ volatile("": "r="(d) :: "memory");
+}
-- 
2.14.1

Re: [PATCH] proposal adding explicit_bzero

[Rich Felker]

On Fri, Jun 15, 2018 at 01:37:43PM +0000, David CARLIER wrote:
> Hi dear lists,
> this is my first message so forgive me if this idea had already been rejected.

It's definitely not rejected outright, and I think the consensus is
to adopt it. But..

> From c0a16cf96b96b009097d6ed656a2a7b8969e8399 Mon Sep 17 00:00:00 2001
> From: David Carlier <dcarlier@afilias.info>
> Date: Fri, 15 Jun 2018 13:30:09 +0000
> Subject: [PATCH] string: adding simple explicit_bzero implementation.
> 
> glibc implementing it and modern security based code starting
> using it widely, here a simple implementation using memory barrier.
> ---
>  include/string.h            | 1 +
>  src/string/explicit_bzero.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
>  create mode 100644 src/string/explicit_bzero.c
> 
> diff --git a/include/string.h b/include/string.h
> index ce1dc300..795a2abc 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -82,6 +82,7 @@ void *memccpy (void *__restrict, const void *__restrict, int, size_t);
>  char *strsep(char **, const char *);
>  size_t strlcat (char *, const char *, size_t);
>  size_t strlcpy (char *, const char *, size_t);
> +void explicit_bzero (void *, size_t);
>  #endif
>  
>  #ifdef _GNU_SOURCE
> diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
> new file mode 100644
> index 00000000..47dba3c7
> --- /dev/null
> +++ b/src/string/explicit_bzero.c
> @@ -0,0 +1,8 @@
> +#define _BSD_SOURCE
> +#include <string.h>
> +
> +void explicit_bzero(void *d, size_t n)
> +{
> +	memset(d, 0, n);
> +	__asm__ volatile("": "r="(d) :: "memory");
> +}
> -- 

The constraint here looks wrong. Normally = is written before the
type, not after; I'm not sure if all compiler versions accept the
unusual form with it after. But more importantly you have it as an
output constraint, where it's essentially a dead store, such that the
asm block does nothing to make explicit_bzero force the memset to
happen.

I think you meant for the constraint to be an input constraint "r"(d).
Does that sound right?

Rich

Re: [PATCH] proposal adding explicit_bzero

[David CARLIER]

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

Yes true sorry :-|.

Kind regards.

On Tue, 26 Jun 2018 at 21:43, Rich Felker <dalias@libc.org> wrote:

> On Fri, Jun 15, 2018 at 01:37:43PM +0000, David CARLIER wrote:
> > Hi dear lists,
> > this is my first message so forgive me if this idea had already been
> rejected.
>
> It's definitely not rejected outright, and I think the consensus is
> to adopt it. But..
>
> > From c0a16cf96b96b009097d6ed656a2a7b8969e8399 Mon Sep 17 00:00:00 2001
> > From: David Carlier <dcarlier@afilias.info>
> > Date: Fri, 15 Jun 2018 13:30:09 +0000
> > Subject: [PATCH] string: adding simple explicit_bzero implementation.
> >
> > glibc implementing it and modern security based code starting
> > using it widely, here a simple implementation using memory barrier.
> > ---
> >  include/string.h            | 1 +
> >  src/string/explicit_bzero.c | 8 ++++++++
> >  2 files changed, 9 insertions(+)
> >  create mode 100644 src/string/explicit_bzero.c
> >
> > diff --git a/include/string.h b/include/string.h
> > index ce1dc300..795a2abc 100644
> > --- a/include/string.h
> > +++ b/include/string.h
> > @@ -82,6 +82,7 @@ void *memccpy (void *__restrict, const void
> *__restrict, int, size_t);
> >  char *strsep(char **, const char *);
> >  size_t strlcat (char *, const char *, size_t);
> >  size_t strlcpy (char *, const char *, size_t);
> > +void explicit_bzero (void *, size_t);
> >  #endif
> >
> >  #ifdef _GNU_SOURCE
> > diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
> > new file mode 100644
> > index 00000000..47dba3c7
> > --- /dev/null
> > +++ b/src/string/explicit_bzero.c
> > @@ -0,0 +1,8 @@
> > +#define _BSD_SOURCE
> > +#include <string.h>
> > +
> > +void explicit_bzero(void *d, size_t n)
> > +{
> > +     memset(d, 0, n);
> > +     __asm__ volatile("": "r="(d) :: "memory");
> > +}
> > --
>
> The constraint here looks wrong. Normally = is written before the
> type, not after; I'm not sure if all compiler versions accept the
> unusual form with it after. But more importantly you have it as an
> output constraint, where it's essentially a dead store, such that the
> asm block does nothing to make explicit_bzero force the memset to
> happen.
>
> I think you meant for the constraint to be an input constraint "r"(d).
> Does that sound right?
>
> Rich
>

[-- Attachment #2: Type: text/html, Size: 3005 bytes --]

Re: [PATCH] proposal adding explicit_bzero

[Rich Felker]

On Tue, Jun 26, 2018 at 09:48:47PM +0100, David CARLIER wrote:
> Yes true sorry :-|.

No problem. Do you agree with my proposed fix and want me to make that
change and apply it?

Rich


> On Tue, 26 Jun 2018 at 21:43, Rich Felker <dalias@libc.org> wrote:
> 
> > On Fri, Jun 15, 2018 at 01:37:43PM +0000, David CARLIER wrote:
> > > Hi dear lists,
> > > this is my first message so forgive me if this idea had already been
> > rejected.
> >
> > It's definitely not rejected outright, and I think the consensus is
> > to adopt it. But..
> >
> > > From c0a16cf96b96b009097d6ed656a2a7b8969e8399 Mon Sep 17 00:00:00 2001
> > > From: David Carlier <dcarlier@afilias.info>
> > > Date: Fri, 15 Jun 2018 13:30:09 +0000
> > > Subject: [PATCH] string: adding simple explicit_bzero implementation.
> > >
> > > glibc implementing it and modern security based code starting
> > > using it widely, here a simple implementation using memory barrier.
> > > ---
> > >  include/string.h            | 1 +
> > >  src/string/explicit_bzero.c | 8 ++++++++
> > >  2 files changed, 9 insertions(+)
> > >  create mode 100644 src/string/explicit_bzero.c
> > >
> > > diff --git a/include/string.h b/include/string.h
> > > index ce1dc300..795a2abc 100644
> > > --- a/include/string.h
> > > +++ b/include/string.h
> > > @@ -82,6 +82,7 @@ void *memccpy (void *__restrict, const void
> > *__restrict, int, size_t);
> > >  char *strsep(char **, const char *);
> > >  size_t strlcat (char *, const char *, size_t);
> > >  size_t strlcpy (char *, const char *, size_t);
> > > +void explicit_bzero (void *, size_t);
> > >  #endif
> > >
> > >  #ifdef _GNU_SOURCE
> > > diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
> > > new file mode 100644
> > > index 00000000..47dba3c7
> > > --- /dev/null
> > > +++ b/src/string/explicit_bzero.c
> > > @@ -0,0 +1,8 @@
> > > +#define _BSD_SOURCE
> > > +#include <string.h>
> > > +
> > > +void explicit_bzero(void *d, size_t n)
> > > +{
> > > +     memset(d, 0, n);
> > > +     __asm__ volatile("": "r="(d) :: "memory");
> > > +}
> > > --
> >
> > The constraint here looks wrong. Normally = is written before the
> > type, not after; I'm not sure if all compiler versions accept the
> > unusual form with it after. But more importantly you have it as an
> > output constraint, where it's essentially a dead store, such that the
> > asm block does nothing to make explicit_bzero force the memset to
> > happen.
> >
> > I think you meant for the constraint to be an input constraint "r"(d).
> > Does that sound right?
> >
> > Rich
> >

Re: [PATCH] proposal adding explicit_bzero

[David CARLIER]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

Sure I do agree. Thanks.

On Tue, 26 Jun 2018 at 21:53, Rich Felker <dalias@libc.org> wrote:

> On Tue, Jun 26, 2018 at 09:48:47PM +0100, David CARLIER wrote:
> > Yes true sorry :-|.
>
> No problem. Do you agree with my proposed fix and want me to make that
> change and apply it?
>
> Rich
>
>
> > On Tue, 26 Jun 2018 at 21:43, Rich Felker <dalias@libc.org> wrote:
> >
> > > On Fri, Jun 15, 2018 at 01:37:43PM +0000, David CARLIER wrote:
> > > > Hi dear lists,
> > > > this is my first message so forgive me if this idea had already been
> > > rejected.
> > >
> > > It's definitely not rejected outright, and I think the consensus is
> > > to adopt it. But..
> > >
> > > > From c0a16cf96b96b009097d6ed656a2a7b8969e8399 Mon Sep 17 00:00:00
> 2001
> > > > From: David Carlier <dcarlier@afilias.info>
> > > > Date: Fri, 15 Jun 2018 13:30:09 +0000
> > > > Subject: [PATCH] string: adding simple explicit_bzero implementation.
> > > >
> > > > glibc implementing it and modern security based code starting
> > > > using it widely, here a simple implementation using memory barrier.
> > > > ---
> > > >  include/string.h            | 1 +
> > > >  src/string/explicit_bzero.c | 8 ++++++++
> > > >  2 files changed, 9 insertions(+)
> > > >  create mode 100644 src/string/explicit_bzero.c
> > > >
> > > > diff --git a/include/string.h b/include/string.h
> > > > index ce1dc300..795a2abc 100644
> > > > --- a/include/string.h
> > > > +++ b/include/string.h
> > > > @@ -82,6 +82,7 @@ void *memccpy (void *__restrict, const void
> > > *__restrict, int, size_t);
> > > >  char *strsep(char **, const char *);
> > > >  size_t strlcat (char *, const char *, size_t);
> > > >  size_t strlcpy (char *, const char *, size_t);
> > > > +void explicit_bzero (void *, size_t);
> > > >  #endif
> > > >
> > > >  #ifdef _GNU_SOURCE
> > > > diff --git a/src/string/explicit_bzero.c
> b/src/string/explicit_bzero.c
> > > > new file mode 100644
> > > > index 00000000..47dba3c7
> > > > --- /dev/null
> > > > +++ b/src/string/explicit_bzero.c
> > > > @@ -0,0 +1,8 @@
> > > > +#define _BSD_SOURCE
> > > > +#include <string.h>
> > > > +
> > > > +void explicit_bzero(void *d, size_t n)
> > > > +{
> > > > +     memset(d, 0, n);
> > > > +     __asm__ volatile("": "r="(d) :: "memory");
> > > > +}
> > > > --
> > >
> > > The constraint here looks wrong. Normally = is written before the
> > > type, not after; I'm not sure if all compiler versions accept the
> > > unusual form with it after. But more importantly you have it as an
> > > output constraint, where it's essentially a dead store, such that the
> > > asm block does nothing to make explicit_bzero force the memset to
> > > happen.
> > >
> > > I think you meant for the constraint to be an input constraint "r"(d).
> > > Does that sound right?
> > >
> > > Rich
> > >
>

[-- Attachment #2: Type: text/html, Size: 3961 bytes --]