I'd say it is good to go, but some minor stuff for consideration:

1) Is including HTML and PDF manuals in 'make dist' tarballs still
useful?  Reproducing the tarballs from git requires more tools,
especially the PDF.  Removing them reduces tarball from 2.7MB to 2.0MB.

2) The tarball embeds some username info, some
portability/reproducability TAR_OPTIONS for inspiration:

export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner --sort=name 
--mode=go+u,go-w --format=ustar

3) On a riscv64 machine (RevyOS, derived from Debian trixie, with gcc
--version 'gcc (Debian 14.3.0-10revyos2) 14.3.0') I got the warning
below, and I'm not sure why it doesn't show up on amd64.  Self-tests
passes fine on the platform.

In function ‘xalloc’,
    inlined from ‘test_aead_message’ at testutils.c:989:19:
testutils.c:37:13: warning: argument 1 value ‘18446744073709551615’ exceeds 
maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
   37 |   void *p = malloc(size);
      |             ^~~~~~~~~~~~
In file included from testutils.h:11,
                 from testutils.c:3:
/usr/include/stdlib.h: In function ‘test_aead_message’:
/usr/include/stdlib.h:672:14: note: in a call to allocation function ‘malloc’ 
declared here
  672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__
      |              ^~~~~~

4) Enabling -fanalyzer shows the warning below.  I've had mixed
experiences with -fanalyzer, but I've had real positives with it.

gcc -I. -I. -Wall -Warith-conversion -Wcast-align=strict -Wdate-time 
-Wdouble-promotion -Wduplicated-branches -Wduplicated-cond -Wextra 
-Wformat-signedness -Wflex-array-member-not-at-end -Winit-self -Winvalid-pch 
-Wlogical-op -Wmissing-include-dirs -Wnull-dereference -Wold-style-definition 
-Wopenmp-simd -Woverlength-strings -Wpacked -Wpointer-arith -Wstack-protector 
-Wstrict-prototypes -Wsuggest-attribute=cold -Wsuggest-final-methods 
-Wsuggest-final-types -Wsync-nand -Wtrampolines -Wuninitialized 
-Wunknown-pragmas -Wunsafe-loop-optimizations -Wunused-macros -Wvariadic-macros 
-Wvector-operation-performance -Wvla -Wwrite-strings -Warray-bounds=2 
-Wattribute-alias=2 -Wbidi-chars=any,ucn -Wformat-overflow=2 -Wformat=2 
-Wformat-truncation=2 -Wimplicit-fallthrough=5 -Wshift-overflow=2 
-Wuse-after-free=3 -Wunused-const-variable=2 -Wvla-larger-than=4031 
-Wno-analyzer-malloc-leak -Wno-sign-compare -Wno-system-headers -Wno-cast-align 
-Wno-clobbered -Wno-discarded-qualifiers -Wno-format -Wno-implicit-fallthrough 
-Wno-unused-parameter -fstrict-flex-arrays -Wstrict-flex-arrays -Werror 
-Wno-error=unused-macros -Wno-error=unused-const-variable= 
-Wno-error=format-truncation -fanalyzer -DHAVE_CONFIG_H -g -O2 -ggdb3 -Wall -W 
-Wno-sign-compare   -Wmissing-prototypes -Wmissing-declarations 
-Wstrict-prototypes   -Wpointer-arith -Wbad-function-cast -Wnested-externs 
-fpic -MT gosthash94.o -MD -MP -MF gosthash94.o.d -c gosthash94.c \
&& true
gosthash94.c: In function ‘gost_block_compress’:
gosthash94.c:76:40: error: use of uninitialized value ‘v[1]’ [CWE-457] 
[-Werror=analyzer-use-of-uninitialized-value]
   76 |     w[0] = u[0] ^ v[0], w[1] = u[1] ^ v[1];
      |                                       ~^~~
  ‘gosthash94_update_int.part.0’: event 1
    |
    |  297 | gosthash94_update_int (struct gosthash94_ctx *ctx,
    |      | ^~~~~~~~~~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘gosthash94_update_int.part.0’
    |
  ‘gosthash94_update_int.part.0’: event 2
    |
    |macros.h:184:8:
    |  184 |     if ((ctx)->index)                                              
     \
    |      |        ^
    |      |        |
    |      |        (2) following ‘false’ branch...
gosthash94.c:301:5: note: in expansion of macro ‘MD_UPDATE’
    |  301 |     MD_UPDATE(ctx, length, msg, COMPRESS, ctx->count++);
    |      |     ^~~~~~~~~
    |
  ‘gosthash94_update_int.part.0’: event 3
    |
    |cc1:
    | (3): ...to here
    |
  ‘gosthash94_update_int.part.0’: event 4
    |
    |macros.h:205:21:
    |  205 |     while ((length) >= sizeof((ctx)->block))                       
     \
    |      |            ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
    |      |                     |
    |      |                     (4) following ‘true’ branch (when ‘length > 
31’)...
gosthash94.c:301:5: note: in expansion of macro ‘MD_UPDATE’
    |  301 |     MD_UPDATE(ctx, length, msg, COMPRESS, ctx->count++);
    |      |     ^~~~~~~~~
    |
  ‘gosthash94_update_int.part.0’: event 5
    |
    |  286 | #define COMPRESS(ctx, block) gost_compute_sum_and_hash((ctx), 
(block), sbox);
    |      |                              
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                              |
    |      |                              (5) ...to here
macros.h:207:9: note: in expansion of macro ‘COMPRESS’
    |  207 |         f((ctx), (data));                                          
     \
    |      |         ^
gosthash94.c:301:5: note: in expansion of macro ‘MD_UPDATE’
    |  301 |     MD_UPDATE(ctx, length, msg, COMPRESS, ctx->count++);
    |      |     ^~~~~~~~~
    |
  ‘gosthash94_update_int.part.0’: event 6
    |
    |  286 | #define COMPRESS(ctx, block) gost_compute_sum_and_hash((ctx), 
(block), sbox);
    |      |                              
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                              |
    |      |                              (6) calling 
‘gost_compute_sum_and_hash’ from ‘gosthash94_update_int.part.0’
macros.h:207:9: note: in expansion of macro ‘COMPRESS’
    |  207 |         f((ctx), (data));                                          
     \
    |      |         ^
gosthash94.c:301:5: note: in expansion of macro ‘MD_UPDATE’
    |  301 |     MD_UPDATE(ctx, length, msg, COMPRESS, ctx->count++);
    |      |     ^~~~~~~~~
    |
    +--> ‘gost_compute_sum_and_hash’: events 7-8
           |
           |  266 | gost_compute_sum_and_hash (struct gosthash94_ctx *ctx, 
const uint8_t *block,
           |      | ^~~~~~~~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (7) entry to ‘gost_compute_sum_and_hash’
           |......
           |  273 |     for (i = carry = 0; i < 8; i++, block += 4)
           |      |                         ~~~~~
           |      |                           |
           |      |                           (8) following ‘true’ branch (when 
‘i != 8’)...
           |
         ‘gost_compute_sum_and_hash’: event 9
           |
           |macros.h:120:20:
           |  120 | (  (((uint32_t) (p)[3]) << 24)                  \
           |      |                 ~~~^~~
           |      |                    |
           |      |                    (9) ...to here
gosthash94.c:275:25: note: in expansion of macro ‘LE_READ_UINT32’
           |  275 |           block_le[i] = LE_READ_UINT32(block);
           |      |                         ^~~~~~~~~~~~~~
           |
         ‘gost_compute_sum_and_hash’: events 10-12
           |
           |  273 |     for (i = carry = 0; i < 8; i++, block += 4)
           |      |                         ~~^~~
           |      |                           |
           |      |                           (10) following ‘false’ branch 
(when ‘i == 8’)...
           |......
           |  283 |     gost_block_compress (ctx, block_le, sbox);
           |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |     |
           |      |     (11) ...to here
           |      |     (12) calling ‘gost_block_compress’ from 
‘gost_compute_sum_and_hash’
           |
           +--> ‘gost_block_compress’: events 13-15
                  |
                  |   65 | gost_block_compress (struct gosthash94_ctx *ctx, 
const uint32_t *block,
                  |      | ^~~~~~~~~~~~~~~~~~~
                  |      | |
                  |      | (13) entry to ‘gost_block_compress’
                  |......
                  |   69 |     uint32_t key[8], u[8], v[8], w[8], s[8];
                  |      |                            ~
                  |      |                            |
                  |      |                            (14) region created on 
stack here
                  |......
                  |   76 |     w[0] = u[0] ^ v[0], w[1] = u[1] ^ v[1];
                  |      |                                       ~~~~
                  |      |                                        |
                  |      |                                        (15) use of 
uninitialized value ‘v[1]’ here
                  |
cc1: all warnings being treated as errors

5) Another warning, just for testsuite:

poly1305-test.c: In function ‘test_fixed’:
poly1305-test.c:184:7: error: ‘test_poly1305_internal’ reading 16 bytes from a 
region of size 1 [-Werror=stringop-overread]
  184 |       test_poly1305_internal (key->data, message->length, 
message->data, nonce, digest->data);
      |       
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
poly1305-test.c:184:7: note: referencing argument 1 of type ‘const uint8_t[16]’ 
{aka ‘const unsigned char[16]’}
poly1305-test.c:184:7: note: referencing argument 4 of type ‘const uint8_t[16]’ 
{aka ‘const unsigned char[16]’}
poly1305-test.c:184:7: error: ‘test_poly1305_internal’ reading 16 bytes from a 
region of size 1 [-Werror=stringop-overread]
poly1305-test.c:184:7: note: referencing argument 5 of type ‘const uint8_t[16]’ 
{aka ‘const unsigned char[16]’}
poly1305-test.c:127:1: note: in a call to function ‘test_poly1305_internal’
  127 | test_poly1305_internal (const uint8_t key[16],
      | ^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

6) Sometimes -fstrict-flex-arrays -Wstrict-flex-arrays finds interesting
spots too --
https://blogs.oracle.com/linux/closing-a-hole-in-the-detection-of-buffer-overflows-with-gcc
-- also for testsuite

gcc -I.. -I.. -Wall -Warith-conversion -Wcast-align=strict -Wdate-time 
-Wdouble-promotion -Wduplicated-branches -Wduplicated-cond -Wextra 
-Wformat-signedness -Wflex-array-member-not-at-end -Winit-self -Winvalid-pch 
-Wlogical-op -Wmissing-include-dirs -Wnull-dereference -Wold-style-definition 
-Wopenmp-simd -Woverlength-strings -Wpacked -Wpointer-arith -Wstack-protector 
-Wstrict-prototypes -Wsuggest-attribute=cold -Wsuggest-final-methods 
-Wsuggest-final-types -Wsync-nand -Wtrampolines -Wuninitialized 
-Wunknown-pragmas -Wunsafe-loop-optimizations -Wunused-macros -Wvariadic-macros 
-Wvector-operation-performance -Wvla -Wwrite-strings -Warray-bounds=2 
-Wattribute-alias=2 -Wbidi-chars=any,ucn -Wformat-overflow=2 -Wformat=2 
-Wformat-truncation=2 -Wimplicit-fallthrough=5 -Wshift-overflow=2 
-Wuse-after-free=3 -Wunused-const-variable=2 -Wvla-larger-than=4031 
-Wno-analyzer-malloc-leak -Wno-sign-compare -Wno-system-headers -Wno-cast-align 
-Wno-clobbered -Wno-discarded-qualifiers -Wno-format -Wno-implicit-fallthrough 
-Wno-unused-parameter -fstrict-flex-arrays -Wstrict-flex-arrays -Werror 
-Wno-error=unused-macros -Wno-error=unused-const-variable= 
-Wno-error=format-truncation -fanalyzer -Wno-error=overlength-strings 
-DHAVE_CONFIG_H -g -O2 -ggdb3 -Wall -W -Wno-sign-compare   -Wmissing-prototypes 
-Wmissing-declarations -Wstrict-prototypes   -Wpointer-arith 
-Wbad-function-cast -Wnested-externs  -MT pss-test.o -MD -MP -MF pss-test.o.d 
-c pss-test.c && true
pss-test.c: In function ‘test_main’:
pss-test.c:57:13: error: array subscript 6 is above array bounds of 
‘uint8_t[1]’ {aka ‘unsigned char[1]’} [-Werror=array-bounds=]
   57 |   salt->data[6] = 0x00;
      |   ~~~~~~~~~~^~~
pss-test.c:57:13: error: trailing array ‘uint8_t[1]’ {aka ‘unsigned char[1]’} 
should not be used as a flexible array member [-Werror=strict-flex-arrays]

/Simon

Attachment: signature.asc
Description: PGP signature

_______________________________________________
nettle-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to