[LTP] [PATCH v2] lib: Handle empty saved configuration values

[Martin Doucha]

Hi,
looks good.

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 9/30/26 09:43, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
> 
> Empty string module parameters return EOF on kernels prior to 4.14
> commit 96802e6b1dbf ("kernel/params.c: fix an overflow in
> param_attr_show"), causing fw_load02 to abort with TBROK on v4.4.
> 
> Accept empty values while retaining read-error handling and the original
> errno. Restore empty strings with a NUL byte: an empty write does not
> invoke the sysfs setter, while a newline would change the saved value.
> 
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> Fix the following behaviour on kernel < 4.14 96802e6b1dbf
> 
> tst_kconfig.c:90: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:90: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:90: TINFO: Parsing kernel config '/proc/config.gz'
> tst_security.c:115: TINFO: SecureBoot sysfs file not available
> tst_security.c:94: TINFO: Unable to determine system lockdown state
> tst_security.c:115: TINFO: SecureBoot sysfs file not available
> tst_tmpdir.c:309: TINFO: Using /tmp/LTP_fw_ZRzLML as tmpdir (btrfs filesystem)
> tst_sys_conf.c:103: TBROK: Failed to read anything from '/sys/module/firmware_class/parameters/path': SUCCESS (0)
> 
> Summary:
> passed   0
> failed   0
> broken   1
> skipped  0
> warnings 0
> ---
> Changes in v2:
> - fix commit message
> - fix according to Martin's suggestion
> - Link to v1: https://lore.kernel.org/20260929-fix_fw_load02-v1-1-4cd5f95d172b@suse.com
> ---
>   lib/tst_sys_conf.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
> index 80cd83569..f69eb3c7f 100644
> --- a/lib/tst_sys_conf.c
> +++ b/lib/tst_sys_conf.c
> @@ -54,10 +54,10 @@ void tst_sys_conf_save_str(const char *path, const char *value)
>   
>   int tst_sys_conf_save(const struct tst_path_val *conf)
>   {
> -	char line[PATH_MAX];
> +	char line[PATH_MAX] = {};
>   	int ttype, iret;
> +	int read_err = 0, saved_errno = 0;
>   	FILE *fp;
> -	void *ret;
>   
>   	if (!conf || !conf->path)
>   		tst_brk(TBROK, "path is empty");
> @@ -92,13 +92,17 @@ int tst_sys_conf_save(const struct tst_path_val *conf)
>   		return 1;
>   	}
>   
> -	ret = fgets(line, sizeof(line), fp);
> +	if (!fgets(line, sizeof(line), fp))
> +		read_err = ferror(fp);
> +
> +	saved_errno = errno;
>   	fclose(fp);
>   
> -	if (ret == NULL) {
> +	if (read_err) {
>   		if (conf->flags & TST_SR_IGNORE_ERR)
>   			return 1;
>   
> +		errno = saved_errno;
>   		tst_brk(TBROK | TERRNO, "Failed to read anything from '%s'",
>   			conf->path);
>   	}
> @@ -142,7 +146,14 @@ void tst_sys_conf_restore(int verbose)
>   			tst_res(TINFO, "Restoring conf.: %s -> %s\n",
>   				i->path, i->value);
>   		}
> -		FILE_PRINTF(i->path, "%s", i->value);
> +		/*
> +		 * A zero-length write does not invoke the sysfs store callback.
> +		 * Write a NUL byte to restore an empty string module parameter.
> +		 */
> +		if (!i->value[0])
> +			FILE_PRINTF(i->path, "%c", '\0');
> +		else
> +			FILE_PRINTF(i->path, "%s", i->value);
>   	}
>   }
>   
> 
> ---
> base-commit: 1cbe713ca9d55bed7e4d52c6f2216ba3e2089346
> change-id: 20260929-fix_fw_load02-6b0305f6bd7b
> 
> Best regards,
> --
> Andrea Cervesato <andrea.cervesato@suse.com>
> 
> 


-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic