[PATCH] fix gcc string bound error

[Michael Haener]

Avoid the "stringop-overflow" error. The so far specified bound
depends on the length of the source argument.

Signed-off-by: Michael Haener <michael...@siemens.com>
---
env/env_config_partitions.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
index f4b17e2..3b211e6 100644
--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -61,7 +61,7 @@ bool probe_config_partitions(CONFIG_PART *cfgpart)
return false;
}
}
- strncpy(cfgpart[count].devpath, devpath,
+ memcpy(cfgpart[count].devpath, devpath,
strlen(devpath) + 1);
if (probe_config_file(&cfgpart[count])) {
printf_debug("%s", "Environment file found.\n");
--
2.26.3

[Jan Kiszka]

The change makes sense, specifically when looking at the code above it
that allocates cfgpart[count].devpath accordingly.

However, as we are at it: The condition for allocating looks suspicious.
We only do so when devpath wasn't allocated yet. What could cause that
case? Are we then sure that devpath was allocated with the right size?
Very likely, because we are still on the same system, and bgenv_init
calls probe_config_partitions only once while bgenv_finalize properly
cleans up afterwards. All that is likely needlessly convoluted.

So, bonus for sorting /that/ out as well.

Thanks, this one merged already.

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

[Haener, Michael]

I have also looked at the allocation. I came to a similar conclusion,
but I wasn't sure. That's why I only considered copying :-)

Michael

[Jan Kiszka]

[re-adding the list]

On 25.05.21 08:00, Häner, Michael (SI BP R&D ZG FW CCP) wrote:
> I have also looked at the allocation. I came to a similar conclusion,

> but I wasn't sure. That's why I only considered copying.
>

Copying is ok if allocation of the same size happened right before that.
In fact, strdup could actually do both things... Maybe a reason to drop
the patch in favor of a v2 that does exactly that.

Jan

> Michael
>
> On Tue, 2021-05-25 at 07:27 +0200, Jan Kiszka wrote:

[Michael Haener]

Avoid the "stringop-overflow" error. The so far specified bound
depends on the length of the source argument.

Signed-off-by: Michael Haener <michael...@siemens.com>
---

env/env_config_partitions.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
index f4b17e2..db3e08f 100644
--- a/env/env_config_partitions.c
+++ b/env/env_config_partitions.c
@@ -53,16 +53,11 @@ bool probe_config_partitions(CONFIG_PART *cfgpart)
(void)snprintf(devpath, 4096, "%s%u",
dev->path, part->num);
}
+ cfgpart[count].devpath = strdup(devpath);
if (!cfgpart[count].devpath) {
- cfgpart[count].devpath =
- malloc(strlen(devpath) + 1);
- if (!cfgpart[count].devpath) {
- VERBOSE(stderr, "Out of memory.");
- return false;
- }
+ VERBOSE(stderr, "Out of memory.");
+ return false;

}
- strncpy(cfgpart[count].devpath, devpath,

- strlen(devpath) + 1);

if (probe_config_file(&cfgpart[count])) {
printf_debug("%s", "Environment file found.\n");

if (count >= ENV_NUM_CONFIG_PARTS) {
--
2.26.3

[Jan Kiszka]

Thanks, inserted this into next.