On 3/12/25 16:21, Florian Westphal wrote:
> Corubba Smith <corubba@xxxxxx> wrote:
>> Until a6fbeb96e889 ("new configuration file syntax (Magnus Boden)")
>> this was already caught, and the enum member is still present.
>>
>> Check if the for loop worked throught the whole array without hitting a
>> matching config option, and return with the unknown key error code.
>> Because there is no existing config_entry struct with that unknwon key
>> to use with the established config_errce pointer, allocate a new struct.
>> This potentially creates a memory leak if that config_entry is never
>> freed again, but for me that is acceptable is this rare case.
>>
>> Since the memory allocation for the struct can fail, also reuse the old
>> out-of-memory error to indicate that.
>>
>> Signed-off-by: Corubba Smith <corubba@xxxxxx>
>> ---
>> Changes in v2:
>> - Reduce indentation of case statements (Florian Westphal)
>>
>> src/conffile.c | 11 +++++++++++
>> src/ulogd.c | 2 ++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/src/conffile.c b/src/conffile.c
>> index cc5552c..7b9fb0f 100644
>> --- a/src/conffile.c
>> +++ b/src/conffile.c
>> @@ -236,6 +236,17 @@ int config_parse_file(const char *section, struct config_keyset *kset)
>> break;
>> }
>> pr_debug("parse_file: exiting main loop\n");
>> +
>> + if (i == kset->num_ces) {
>> + config_errce = calloc(1, sizeof(struct config_entry));
>> + if (config_errce == NULL) {
>> + err = -ERROOM;
>> + goto cpf_error;
>> + }
>> + strncpy(&config_errce->key[0], wordbuf, CONFIG_KEY_LEN - 1);
>
> This raises a bogus compiler warning for me:
> conffile.c:246:25: warning: '__builtin_strncpy' output may be truncated copying 30 bytes from a string of length 254 [-Wstringop-truncation]
> 246 | strncpy(config_errce->key, wordbuf, sizeof(config_errce->key));
>
> What do you make of this?
>
> - strncpy(&config_errce->key[0], wordbuf, CONFIG_KEY_LEN - 1);
> + snprintf(config_errce->key, sizeof(config_errce->key), "%s", wordbuf);
>
>
Today I learned: GCC will print certain warnings only when certain
optimizations are applied. That warning is only there with -O2 or above,
I was using -Og. That's why I didn't see or catch it.
Your suggestion using snprintf works, but feels a bit heavy-handed.
I like the suggestion in the GCC doc [0] of using memcpy for
(potentially) not-NULL-terminated strings better. Since the target
memory comes from calloc(), the last byte will always be NULL and
properly terminate a potentially truncated string copied from wordbuf.
So just memcpy the 29 bytes before that. It may copy data beyond the
first NULL character in wordbuf, but with a maximum of 29 bytes it's
not a big deal.
So my proposed fix would be:
- strncpy(&config_errce->key[0], wordbuf, CONFIG_KEY_LEN - 1);
+ memcpy(&config_errce->key[0], wordbuf, sizeof(config_errce->key) - 1);
Compiles without a warning, and tested to work as expected. Would you
like a v3 of the whole patch, or is this addendum good enough?
[0] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-truncation
--
Corubba
