Re: [i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory

[Kamil Konieczny <kamil.konieczny@linux.intel.com>]

On 2024-04-12 at 14:20:14 +0200, Zbigniew Kempczyński wrote:
> I've noticed on running kunit subtest:
> 
> ./xe_live_ktest --r xe_bo
> 
> IGT-Version: 1.28-NO-GIT (x86_64) (Linux: 6.8.0-xeint+ x86_64)
> Using IGT_SRANDOM=1712922311 for randomisation
> Starting subtest: xe_bo
> Received signal SIGSEGV.
> Stack trace:
>  #0 [fatal_sig_handler+0xda]
>  #1 [__sigaction+0x50]
>  #2 [__libc_sigaction+0x10f]
>  #3 [kunit_get_tests+0x417]
>  #4 [igt_kunit+0x35f]
>  #5 [__igt_unique____real_main41+0x44]
>  #6 [main+0x48]
>  #7 [__libc_init_first+0x90]
>  #8 [__libc_start_main+0x80]
>  #9 [_start+0x25]
> Subtest xe_bo: CRASH (0.005s)
> 
> Looks this is related to sigaction() write to memory referenced
> by uninitialized pointer located on the stack. Lets fix it.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>

Thx for reporting bug and fixing it.  +cc Janusz

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
> v2: missed cmdline (due to # character)
> ---
>  lib/igt_kmod.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 1ec9c8a602..6659c27eba 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -963,7 +963,7 @@ static int kunit_kmsg_result_get(struct igt_list_head *results,
>  				 int fd, struct igt_ktap_results *ktap)
>  {
>  	struct sigaction sigchld = { .sa_handler = kunit_sigchld_handler, },
> -			 *saved;
> +			 saved;
>  	char record[BUF_LEN + 1], *buf;
>  	unsigned long taints;
>  	int ret;
> @@ -975,7 +975,7 @@ static int kunit_kmsg_result_get(struct igt_list_head *results,
>  			return -ENOTRECOVERABLE;
>  
>  		if (modprobe) {
> -			err = igt_debug_on(sigaction(SIGCHLD, &sigchld, saved));
> +			err = igt_debug_on(sigaction(SIGCHLD, &sigchld, &saved));
>  			if (err == -1)
>  				return -errno;
>  			else if (unlikely(err))
> @@ -988,7 +988,7 @@ static int kunit_kmsg_result_get(struct igt_list_head *results,
>  				igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
>  				__attribute__ ((fallthrough));
>  			case ENOTRECOVERABLE:
> -				igt_debug_on(sigaction(SIGCHLD, saved, NULL));
> +				igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
>  				if (igt_debug_on(modprobe->err))
>  					return modprobe->err;
>  				break;
> @@ -996,7 +996,7 @@ static int kunit_kmsg_result_get(struct igt_list_head *results,
>  				break;
>  			default:
>  				igt_debug("pthread_mutex_lock() error: %d\n", err);
> -				igt_debug_on(sigaction(SIGCHLD, saved, NULL));
> +				igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
>  				return -err;
>  			}
>  		}
> @@ -1005,7 +1005,7 @@ static int kunit_kmsg_result_get(struct igt_list_head *results,
>  
>  		if (modprobe && !err) {	/* pthread_mutex_lock() succeeded */
>  			igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
> -			igt_debug_on(sigaction(SIGCHLD, saved, NULL));
> +			igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
>  		}
>  
>  		if (igt_debug_on(!ret))
> @@ -1236,7 +1236,7 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  			    struct igt_ktap_results **ktap)
>  {
>  	struct sigaction sigalrm = { .sa_handler = kunit_get_tests_timeout, },
> -			 *saved;
> +			 saved;
>  	struct igt_ktap_result *r, *rn;
>  	unsigned long taints;
>  	int flags, err;
> @@ -1263,13 +1263,13 @@ static bool kunit_get_tests(struct igt_list_head *tests,
>  	igt_skip_on(modprobe(tst->kmod, opts));
>  	igt_skip_on(igt_kernel_tainted(&taints));
>  
> -	igt_skip_on(sigaction(SIGALRM, &sigalrm, saved));
> +	igt_skip_on(sigaction(SIGALRM, &sigalrm, &saved));
>  	alarm(10);
>  
>  	err = kunit_get_results(tests, tst->kmsg, ktap);
>  
>  	alarm(0);
> -	igt_debug_on(sigaction(SIGALRM, saved, NULL));
> +	igt_debug_on(sigaction(SIGALRM, &saved, NULL));
>  
>  	igt_skip_on_f(err,
>  		      "KTAP parser failed while getting a list of test cases\n");