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

[Janusz Krzysztofik]

On Monday, 15 April 2024 11:49:53 CEST Kamil Konieczny wrote:
> 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 at intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Acked-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> Thx for reporting bug and fixing it.  +cc Janusz

Yeah, thanks for fixing, and I'm sorry for the bug.  I'm wondering how we 
managed to leave that long (half a year?) with that bug not discovered.

Thanks,
Janusz

> 
> Reviewed-by: Kamil Konieczny <kamil.konieczny at 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");
>