Opened 12 days ago

Closed 7 days ago

#30475 closed defect (fixed)

hs_service.c: compile-time warning with GCC 9.1.1

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 040-must 035-backport
Cc: asn Actual Points: .1
Parent ID: Points: .1
Reviewer: dgoulet Sponsor:

Description

I tried building with GCC 9.1.1 for the first time, and got various warnings.

make[1]: Entering directory '/home/nickm/src/tor-035'
  CC       src/feature/hs/hs_service.o
In file included from ./src/lib/crypt_ops/crypto_rsa.h:21,
                 from ./src/core/or/or.h:32,
                 from src/feature/hs/hs_service.c:11:
In function ‘load_client_keys’,
    inlined from ‘load_service_keys’ at src/feature/hs/hs_service.c:1090:7,
    inlined from ‘hs_service_load_all_keys’ at src/feature/hs/hs_service.c:4010:9:
./src/lib/log/log.h:244:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
  244 |   log_fn_(LOG_WARN, domain, __FUNCTION__, args, ##__VA_ARGS__)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/feature/hs/hs_service.c:1267:7: note: in expansion of macro ‘log_warn’
 1267 |       log_warn(LD_REND, "Client authorization file %s can't be read. "
      |       ^~~~~~~~
src/feature/hs/hs_service.c: In function ‘hs_service_load_all_keys’:
src/feature/hs/hs_service.c:1267:52: note: format string is defined here
 1267 |       log_warn(LD_REND, "Client authorization file %s can't be read. "
      |                                                    ^~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9877: src/feature/hs/hs_service.o] Error 1
  CC       src/feature/hs/core_libtor_app_testing_a-hs_service.o
In file included from ./src/lib/crypt_ops/crypto_rsa.h:21,
                 from ./src/core/or/or.h:32,
                 from src/feature/hs/hs_service.c:11:
In function ‘load_client_keys’,
    inlined from ‘load_service_keys’ at src/feature/hs/hs_service.c:1090:7,
    inlined from ‘hs_service_load_all_keys’ at src/feature/hs/hs_service.c:4010:9:
./src/lib/log/log.h:244:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
  244 |   log_fn_(LOG_WARN, domain, __FUNCTION__, args, ##__VA_ARGS__)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/feature/hs/hs_service.c:1267:7: note: in expansion of macro ‘log_warn’
 1267 |       log_warn(LD_REND, "Client authorization file %s can't be read. "
      |       ^~~~~~~~
src/feature/hs/hs_service.c: In function ‘hs_service_load_all_keys’:
src/feature/hs/hs_service.c:1267:52: note: format string is defined here
 1267 |       log_warn(LD_REND, "Client authorization file %s can't be read. "
      |                                                    ^~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:11111: src/feature/hs/core_libtor_app_testing_a-hs_service.o] Error 1
make[1]: Target 'all-am' not remade because of errors.
make[1]: Leaving directory '/home/nickm/src/tor-035'
make: *** [Makefile:5788: all] Error 2
[1016]$ 

It looks like this is a real bug: when there's something wrong with the client authorization file, we first free and null the file, and only log its contents afterwards.

This appears to affect 0.3.5 and later.

Child Tickets

Change History (7)

comment:1 Changed 11 days ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 11 days ago by nickm

Actual Points: .1
Status: acceptedneeds_review

See branch bug30475_035 with PR at https://github.com/torproject/tor/pull/1013 . Should also merge cleanly into later branches.

comment:3 Changed 9 days ago by dgoulet

Cc: dgoulet removed
Reviewer: dgoulet

comment:4 Changed 8 days ago by dgoulet

Keywords: asn-merge added
Status: needs_reviewmerge_ready

comment:5 Changed 7 days ago by asn

Merged 040 and onwards. Keeping it open for the backports.

comment:6 Changed 7 days ago by asn

Keywords: asn-merge removed

comment:7 Changed 7 days ago by nickm

Milestone: Tor: 0.4.0.x-finalTor: 0.3.5.x-final
Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.5 as well, to prevent us from accumulating too many branches I can't build at home :)

Note: See TracTickets for help on using tickets.