diff mbox

crypt: Use internal names for the SHA-2 block functions

Message ID e9d4595b-01f8-afca-2265-9e2c2ed38d72@redhat.com
State Superseded
Headers show

Commit Message

Florian Weimer Oct. 28, 2016, 12:57 p.m. UTC 
I have not been able to test the SPARC bits so far, but they look 
reasonably safe to me.

Thanks,
Florian

Comments

Adhemerval Zanella Netto Oct. 28, 2016, 1:31 p.m. UTC | #1 
On 28/10/2016 10:57, Florian Weimer wrote:
> I have not been able to test the SPARC bits so far, but they look reasonably safe to me.


> --- a/sysdeps/sparc/sparc64/multiarch/sha256-block.c

> +++ b/sysdeps/sparc/sparc64/multiarch/sha256-block.c

> @@ -1,12 +1,12 @@

>  #include <sparc-ifunc.h>

>  

> -#define sha256_process_block sha256_process_block_generic

> -extern void sha256_process_block_generic (const void *buffer, size_t len,

> -					  struct sha256_ctx *ctx);

> +#define __sha256_process_block __sha256_process_block_generic

> +extern void __sha256_process_block_generic (const void *buffer, size_t len,

> +					    struct sha256_ctx *ctx);

>  

>  #include <crypt/sha256-block.c>

>  

> -#undef sha256_process_block

> +#undef __sha256_process_block

>  

>  extern void __sha256_process_block_crop (const void *buffer, size_t len,

>  					 struct sha256_ctx *ctx);

> @@ -25,6 +25,8 @@ static bool cpu_supports_sha256(int hwcap)

>    return false;

>  }

>  

> -extern void sha256_process_block (const void *buffer, size_t len,

> -				  struct sha256_ctx *ctx);

> -sparc_libc_ifunc(sha256_process_block, cpu_supports_sha256(hwcap) ? __sha256_process_block_crop : sha256_process_block_generic);

> +extern void __sha256_process_block (const void *buffer, size_t len,

> +				    struct sha256_ctx *ctx);

> +sparc_libc_ifunc (__sha256_process_block,

> +		  cpu_supports_sha256(hwcap) ? __sha256_process_block_crop

> +		    : sha256_process_block_generic);


It should __sha256_process_block_generic here.  Also, if we now aiming
for namespace clean shouldn't we also add a conform test for crypt.h
header?
Joseph Myers Oct. 28, 2016, 5:09 p.m. UTC | #2 
On Fri, 28 Oct 2016, Adhemerval Zanella wrote:

> It should __sha256_process_block_generic here.  Also, if we now aiming

> for namespace clean shouldn't we also add a conform test for crypt.h

> header?


crypt.h is not in any standard supported by the conform tests.

It's true that linknamespace considerations apply even for _DEFAULT_SOURCE 
and _GNU_SOURCE - using a function declared under one of those conditions 
should not bring in a reference to a function not so declared.  The 
difficulty in testing this is that it really needs a list of all installed 
headers to be visible in one place (and there's the question of whether we 
consider _DEFAULT_SOURCE to include all headers or not).

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Netto Oct. 28, 2016, 5:56 p.m. UTC | #3 
On 28/10/2016 15:09, Joseph Myers wrote:
> On Fri, 28 Oct 2016, Adhemerval Zanella wrote:

> 

>> It should __sha256_process_block_generic here.  Also, if we now aiming

>> for namespace clean shouldn't we also add a conform test for crypt.h

>> header?

> 

> crypt.h is not in any standard supported by the conform tests.

> 

> It's true that linknamespace considerations apply even for _DEFAULT_SOURCE 

> and _GNU_SOURCE - using a function declared under one of those conditions 

> should not bring in a reference to a function not so declared.  The 

> difficulty in testing this is that it really needs a list of all installed 

> headers to be visible in one place (and there's the question of whether we 

> consider _DEFAULT_SOURCE to include all headers or not).

> 


Right, but how hard would to add a rule in conform to tests for glibc specific
headers (conformtest-headers-GLIBC or conformtest-headers-GNU) and at
first add the crypt.h plus _GNU_SOURCE?
Joseph Myers Oct. 28, 2016, 6:07 p.m. UTC | #4 
On Fri, 28 Oct 2016, Adhemerval Zanella wrote:

> > It's true that linknamespace considerations apply even for _DEFAULT_SOURCE 

> > and _GNU_SOURCE - using a function declared under one of those conditions 

> > should not bring in a reference to a function not so declared.  The 

> > difficulty in testing this is that it really needs a list of all installed 

> > headers to be visible in one place (and there's the question of whether we 

> > consider _DEFAULT_SOURCE to include all headers or not).

> 

> Right, but how hard would to add a rule in conform to tests for glibc specific

> headers (conformtest-headers-GLIBC or conformtest-headers-GNU) and at

> first add the crypt.h plus _GNU_SOURCE?


You'd need the set of headers for linknamespace tests to be bigger than 
the set for conformtest tests (which makes sense anyway - there are 
several standard variants without conformtest expectations, setting up 
such expectations is a lot of work, but listing headers in a standard is 
easy and listing headers plus specifying compilation options is all that's 
needed for linknamespace tests).

But you'd have to hope that the set of headers you specify for _GNU_SOURCE 
is internally consistent.  It's perfectly valid under our namespace rules 
for a <crypt.h> function to call a function in some Linux-specific header, 
for example (and the Linux-specific headers can't be listed in 
conform/Makefile, only sysdeps/unix/sysv/linux/Makefile knows about them).

I wonder about arranging the makefiles so they record each subdirectory's 
list of installed headers as part of the build, the toplevel collects that 
information together, so when the testsuite runs such a list is available.  
Running tests in each subdirectory works for e.g. Zack's tests, where 
testing for each header is independent, but not for things that need to 
see a complete list of all installed headers at once.  That way, conform/ 
could access that list saved during the build, and filter out bits/ 
headers (and any other headers that just give errors, like Zack's tests 
filter out some such headers) to get a list of headers for GNU / DEFAULT 
linknamespace tests.

-- 
Joseph S. Myers
joseph@codesourcery.com
diff mbox

Patch

crypt: Use internal names for the SHA-2 block functions

These functions are externally visible with a static libcrypt
library.

2016-10-28  Florian Weimer  <fweimer@redhat.com>

	* crypt/sha256.c, crypt/sha256-block.c,
	sysdeps/sparc/sparc64/multiarch/sha256-block.c: Rename
	sha256_process_block to __sha256_process_block.
	* crypt/sha512.c, crypt/sha512-block.c,
	sysdeps/sparc/sparc64/multiarch/sha512-block.c: Rename
	sha512_process_block to __sha512_process_block.

diff --git a/crypt/sha256-block.c b/crypt/sha256-block.c
index 8a77096..a44fe01 100644
--- a/crypt/sha256-block.c
+++ b/crypt/sha256-block.c
@@ -3,7 +3,7 @@ 
 /* Process LEN bytes of BUFFER, accumulating context into CTX.
    It is assumed that LEN % 64 == 0.  */
 void
-sha256_process_block (const void *buffer, size_t len, struct sha256_ctx *ctx)
+__sha256_process_block (const void *buffer, size_t len, struct sha256_ctx *ctx)
 {
   const uint32_t *words = buffer;
   size_t nwords = len / sizeof (uint32_t);
diff --git a/crypt/sha256.c b/crypt/sha256.c
index e858f4b..b5497d9 100644
--- a/crypt/sha256.c
+++ b/crypt/sha256.c
@@ -81,8 +81,7 @@  static const uint32_t K[64] =
     0x90befffa, 0xa4506ceb, 0xbef9a3f7, 0xc67178f2
   };
 
-void
-sha256_process_block (const void *, size_t, struct sha256_ctx *);
+void __sha256_process_block (const void *, size_t, struct sha256_ctx *);
 
 /* Initialize structure containing state of computation.
    (FIPS 180-2:5.3.2)  */
@@ -131,7 +130,7 @@  __sha256_finish_ctx (struct sha256_ctx *ctx, void *resbuf)
 #endif
 
   /* Process last bytes.  */
-  sha256_process_block (ctx->buffer, bytes + pad + 8, ctx);
+  __sha256_process_block (ctx->buffer, bytes + pad + 8, ctx);
 
   /* Put result from CTX in first 32 bytes following RESBUF.  */
   for (unsigned int i = 0; i < 8; ++i)
@@ -156,7 +155,7 @@  __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
 
       if (ctx->buflen > 64)
 	{
-	  sha256_process_block (ctx->buffer, ctx->buflen & ~63, ctx);
+	  __sha256_process_block (ctx->buffer, ctx->buflen & ~63, ctx);
 
 	  ctx->buflen &= 63;
 	  /* The regions in the following copy operation cannot overlap.  */
@@ -182,14 +181,14 @@  __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
       if (UNALIGNED_P (buffer))
 	while (len > 64)
 	  {
-	    sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+	    __sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
 	    buffer = (const char *) buffer + 64;
 	    len -= 64;
 	  }
       else
 #endif
 	{
-	  sha256_process_block (buffer, len & ~63, ctx);
+	  __sha256_process_block (buffer, len & ~63, ctx);
 	  buffer = (const char *) buffer + (len & ~63);
 	  len &= 63;
 	}
@@ -204,7 +203,7 @@  __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
       left_over += len;
       if (left_over >= 64)
 	{
-	  sha256_process_block (ctx->buffer, 64, ctx);
+	  __sha256_process_block (ctx->buffer, 64, ctx);
 	  left_over -= 64;
 	  memcpy (ctx->buffer, &ctx->buffer[64], left_over);
 	}
diff --git a/crypt/sha512-block.c b/crypt/sha512-block.c
index c542db1..577839f 100644
--- a/crypt/sha512-block.c
+++ b/crypt/sha512-block.c
@@ -3,7 +3,7 @@ 
 /* Process LEN bytes of BUFFER, accumulating context into CTX.
    It is assumed that LEN % 128 == 0.  */
 void
-sha512_process_block (const void *buffer, size_t len, struct sha512_ctx *ctx)
+__sha512_process_block (const void *buffer, size_t len, struct sha512_ctx *ctx)
 {
   const uint64_t *words = buffer;
   size_t nwords = len / sizeof (uint64_t);
diff --git a/crypt/sha512.c b/crypt/sha512.c
index 47f3f7c..dd2af3c 100644
--- a/crypt/sha512.c
+++ b/crypt/sha512.c
@@ -101,8 +101,8 @@  static const uint64_t K[80] =
     UINT64_C (0x5fcb6fab3ad6faec), UINT64_C (0x6c44198c4a475817)
   };
 
-void
-sha512_process_block (const void *buffer, size_t len, struct sha512_ctx *ctx);
+void __sha512_process_block (const void *buffer, size_t len,
+			     struct sha512_ctx *ctx);
 
 /* Initialize structure containing state of computation.
    (FIPS 180-2:5.3.3)  */
@@ -153,7 +153,7 @@  __sha512_finish_ctx (struct sha512_ctx *ctx, void *resbuf)
 					   (ctx->total[TOTAL128_low] >> 61));
 
   /* Process last bytes.  */
-  sha512_process_block (ctx->buffer, bytes + pad + 16, ctx);
+  __sha512_process_block (ctx->buffer, bytes + pad + 16, ctx);
 
   /* Put result from CTX in first 64 bytes following RESBUF.  */
   for (unsigned int i = 0; i < 8; ++i)
@@ -178,7 +178,7 @@  __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
 
       if (ctx->buflen > 128)
 	{
-	  sha512_process_block (ctx->buffer, ctx->buflen & ~127, ctx);
+	  __sha512_process_block (ctx->buffer, ctx->buflen & ~127, ctx);
 
 	  ctx->buflen &= 127;
 	  /* The regions in the following copy operation cannot overlap.  */
@@ -204,7 +204,7 @@  __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
       if (UNALIGNED_P (buffer))
 	while (len > 128)
 	  {
-	    sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
+	    __sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
 				    ctx);
 	    buffer = (const char *) buffer + 128;
 	    len -= 128;
@@ -212,7 +212,7 @@  __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
       else
 #endif
 	{
-	  sha512_process_block (buffer, len & ~127, ctx);
+	  __sha512_process_block (buffer, len & ~127, ctx);
 	  buffer = (const char *) buffer + (len & ~127);
 	  len &= 127;
 	}
@@ -227,7 +227,7 @@  __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
       left_over += len;
       if (left_over >= 128)
 	{
-	  sha512_process_block (ctx->buffer, 128, ctx);
+	  __sha512_process_block (ctx->buffer, 128, ctx);
 	  left_over -= 128;
 	  memcpy (ctx->buffer, &ctx->buffer[128], left_over);
 	}
diff --git a/sysdeps/sparc/sparc64/multiarch/sha256-block.c b/sysdeps/sparc/sparc64/multiarch/sha256-block.c
index 79966b9..aa57662 100644
--- a/sysdeps/sparc/sparc64/multiarch/sha256-block.c
+++ b/sysdeps/sparc/sparc64/multiarch/sha256-block.c
@@ -1,12 +1,12 @@ 
 #include <sparc-ifunc.h>
 
-#define sha256_process_block sha256_process_block_generic
-extern void sha256_process_block_generic (const void *buffer, size_t len,
-					  struct sha256_ctx *ctx);
+#define __sha256_process_block __sha256_process_block_generic
+extern void __sha256_process_block_generic (const void *buffer, size_t len,
+					    struct sha256_ctx *ctx);
 
 #include <crypt/sha256-block.c>
 
-#undef sha256_process_block
+#undef __sha256_process_block
 
 extern void __sha256_process_block_crop (const void *buffer, size_t len,
 					 struct sha256_ctx *ctx);
@@ -25,6 +25,8 @@  static bool cpu_supports_sha256(int hwcap)
   return false;
 }
 
-extern void sha256_process_block (const void *buffer, size_t len,
-				  struct sha256_ctx *ctx);
-sparc_libc_ifunc(sha256_process_block, cpu_supports_sha256(hwcap) ? __sha256_process_block_crop : sha256_process_block_generic);
+extern void __sha256_process_block (const void *buffer, size_t len,
+				    struct sha256_ctx *ctx);
+sparc_libc_ifunc (__sha256_process_block,
+		  cpu_supports_sha256(hwcap) ? __sha256_process_block_crop
+		    : sha256_process_block_generic);
diff --git a/sysdeps/sparc/sparc64/multiarch/sha512-block.c b/sysdeps/sparc/sparc64/multiarch/sha512-block.c
index 0d1c3dd..2863e05 100644
--- a/sysdeps/sparc/sparc64/multiarch/sha512-block.c
+++ b/sysdeps/sparc/sparc64/multiarch/sha512-block.c
@@ -1,12 +1,12 @@ 
 #include <sparc-ifunc.h>
 
-#define sha512_process_block sha512_process_block_generic
-extern void sha512_process_block_generic (const void *buffer, size_t len,
-					  struct sha512_ctx *ctx);
+#define __sha512_process_block __sha512_process_block_generic
+extern void __sha512_process_block_generic (const void *buffer, size_t len,
+					    struct sha512_ctx *ctx);
 
 #include <crypt/sha512-block.c>
 
-#undef sha512_process_block
+#undef __sha512_process_block
 
 extern void __sha512_process_block_crop (const void *buffer, size_t len,
 					 struct sha512_ctx *ctx);
@@ -25,6 +25,8 @@  static bool cpu_supports_sha512(int hwcap)
   return false;
 }
 
-extern void sha512_process_block (const void *buffer, size_t len,
-				  struct sha512_ctx *ctx);
-sparc_libc_ifunc(sha512_process_block, cpu_supports_sha512(hwcap) ? __sha512_process_block_crop : sha512_process_block_generic);
+extern void __sha512_process_block (const void *buffer, size_t len,
+				    struct sha512_ctx *ctx);
+sparc_libc_ifunc (__sha512_process_block,
+		  cpu_supports_sha512(hwcap) ? __sha512_process_block_crop
+		    : __sha512_process_block_generic);