[PATCH 0/3] fixes in preperation to move to gcc-11 in CI

[Tyrel Datwyler]

Two fixes for string truncation warnings/errors reported by gcc-[11/12]
toolchains, and a trivial fix to bring pathname buffer allocations in line with
maximum path length.

Tyrel Datwyler (3):
serv_config: cast param to correct size before byte swap
errinjct: pass full device reg path to get_config_addr_from_reg()
errinjct: increase BUFSZ to match PATH_MAX defined by Linux

src/errinjct/ioa_bus_error.c | 14 ++++++--------
src/serv_config.c | 2 +-
2 files changed, 7 insertions(+), 9 deletions(-)

--
2.39.0

[Tyrel Datwyler]

The following string truncation error is reported by gcc-11 and gcc-12
toolchains:

In file included from /usr/powerpc-linux-gnu/include/string.h:535,
from src/errinjct/ioa_bus_error.c:34:

In function ‘strncpy’,
inlined from ‘get_config_addr_from_reg’ at src/errinjct/ioa_bus_error.c:207:2,
inlined from ‘hunt_loc_code’ at src/errinjct/ioa_bus_error.c:415:9:
Warning: /usr/powerpc-linux-gnu/include/bits/string_fortified.h:95:10: warning: ‘__builtin_strncpy’ output may be truncated copying 3995 bytes from a string of length 3999 [-Wstringop-truncation]

This is the result of the caller defining a buffer of BUFSZ, but the
callee only copying BUFSZ-5 of data into a new string so that there is
room to strcat "/reg" to the resulting pathname. We can save a strncpy
and static buffer allocation by doing the strcat of "/reg" to the
pathname before calling get_conig_addr_from_reg() which in general
appears to be inlined anyways.

Reported-by: John Paul Adrian Glaubitz <glau...@physik.fu-berlin.de>
Signed-off-by: Tyrel Datwyler <tyr...@linux.ibm.com>
---
src/errinjct/ioa_bus_error.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/errinjct/ioa_bus_error.c b/src/errinjct/ioa_bus_error.c
index 9d85cfa6875c..94662aa42d87 100644
--- a/src/errinjct/ioa_bus_error.c
+++ b/src/errinjct/ioa_bus_error.c
@@ -194,20 +194,16 @@ int ioa_bus_error_arg(char arg, char *optarg)
* Given the directory /proc/device-tree/pci@...,
* yank out the config address out of the reg file
*
- * @param devpath device-tree path of the reg file
+ * @param regpath device-tree path of the reg file
* @return 0 on failure, config address (!0) on success
*/
-static uint32_t get_config_addr_from_reg(char *devpath)
+static uint32_t get_config_addr_from_reg(char *regpath)
{
- char path[BUFSZ];
char *buf;
uint32_t *be_caddr;
uint32_t caddr = 0;

- strncpy(path, devpath, BUFSZ-5);
- strcat(path, "/reg");
-
- buf = read_file(path, NULL);
+ buf = read_file(regpath, NULL);
if (!buf)
return 1;

@@ -272,6 +268,7 @@ static int parse_sysfsname(void)
/* Obtain the config address from the device-tree reg file */
strcpy(path, "/proc/device-tree/");
strcat(path, devspec);
+ strcat(path, "/reg");
addr = get_config_addr_from_reg(path);
if (addr) {
config_addr = addr;
@@ -412,6 +409,7 @@ static int hunt_loc_code(void)
phb_id_lo = phb_id & 0xFFFFFFFF;

/* Try to get the config address from the dev-tree reg file. */
+ strcat(path, "/reg");
addr = get_config_addr_from_reg(path);
if (addr) {
config_addr = addr;
--
2.39.0

[Tyrel Datwyler]

The first two bytes of the param buffer returned by the
ibm,get-system-parameter RTAS call contain the the length of the
remaining data in the buffer. However, param is a char buffer and
as such the current code attempts to use be16toh which only gets one
byte of data. Type cast param to (uint16_t *) before the dereference to
ensure we get both bytes of data. This fixes the following string
truncation error with gcc-11 and gcc-12 toolchains:

In file included from /usr/powerpc-linux-gnu/include/string.h:535,

from src/serv_config.c:50:
In function ‘strncpy’,
inlined from ‘retrieve_value’ at src/serv_config.c:710:5:
Error: /usr/powerpc-linux-gnu/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ output may be truncated copying between 0 and 255 bytes from a string of length 4997 [-Werror=stringop-truncation]

Reported-by: John Paul Adrian Glaubitz <glau...@physik.fu-berlin.de>
Signed-off-by: Tyrel Datwyler <tyr...@linux.ibm.com>
---

src/serv_config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/serv_config.c b/src/serv_config.c
index 00ab672c1af5..23f90909b5b5 100644
--- a/src/serv_config.c
+++ b/src/serv_config.c
@@ -687,7 +687,7 @@ retrieve_value(struct service_var *var, char *buf, size_t size) {
if (var->sysparm_num == USE_CALL_HOME_SYSPARM)
break;

- ret_size = be16toh(*param);
+ ret_size = be16toh(*((uint16_t *)param));
if (!strcmp(var->nvram_var, "sp-ri-pon") ||
!strcmp(var->nvram_var, "sp-remote-pon") ||
!strcmp(var->nvram_var, "sp-sen")) {
--
2.39.0

[Tyrel Datwyler]

The arbitary BUFSZ macro of 4000 is less than that of PATH_MAX defined
by Linux. BUFSZ is only used for pathname buffer allocations. As such
increase our macro to 4096 to match accordingly.

Signed-off-by: Tyrel Datwyler <tyr...@linux.ibm.com>
---

src/errinjct/ioa_bus_error.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/errinjct/ioa_bus_error.c b/src/errinjct/ioa_bus_error.c
index 94662aa42d87..69778425a9aa 100644
--- a/src/errinjct/ioa_bus_error.c
+++ b/src/errinjct/ioa_bus_error.c
@@ -50,7 +50,7 @@ static char *loc_code; /**< location code of adapter to inject to */

#define IOA_BUSERR_MAXFUNC 19

-#define BUFSZ 4000
+#define BUFSZ 4096

/**
* ioa_buserr_fnames
--
2.39.0

[John Paul Adrian Glaubitz]

Reviewed-by: John Paul Adrian Glaubitz <glau...@physik.fu-berlin.de>

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

[John Paul Adrian Glaubitz]

On 1/19/23 01:27, Tyrel Datwyler wrote:> The following string truncation error is reported by gcc-11 and gcc-12

[Nathan Lynch]

Tyrel Datwyler <tyr...@linux.ibm.com> writes:
> The arbitary BUFSZ macro of 4000 is less than that of PATH_MAX defined
> by Linux. BUFSZ is only used for pathname buffer allocations. As such
> increase our macro to 4096 to match accordingly.

Is there a reason to continue using BUFSZ instead of replacing it with
PATH_MAX throughout the project? There aren't *that* many uses...

[Tyrel Datwyler]

Not really. At least in ioa_bus_error.c we can get rid of the BUFSZ macro
completely. There are some other BUF_SIZE macros which we can't entirely get rid
of since they are used for RTAS buffer sizing, but any where we build a path
should be converted.

-Tyrel