* [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory
@ 2024-04-12 12:20 Zbigniew Kempczyński
2024-04-12 14:15 ` Lucas De Marchi
2024-04-15 9:49 ` [i-g-t] " Kamil Konieczny
0 siblings, 2 replies; 5+ messages in thread
From: Zbigniew Kempczyński @ 2024-04-12 12:20 UTC (permalink / raw)
To: igt-dev; +Cc: Zbigniew Kempczyński, Lucas De Marchi
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>
---
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");
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory
2024-04-12 12:20 [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory Zbigniew Kempczyński
@ 2024-04-12 14:15 ` Lucas De Marchi
2024-04-12 16:20 ` Zbigniew Kempczyński
2024-04-15 9:49 ` [i-g-t] " Kamil Konieczny
1 sibling, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2024-04-12 14:15 UTC (permalink / raw)
To: Zbigniew Kempczyński; +Cc: igt-dev
On Fri, Apr 12, 2024 at 02:20:14PM +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>
not idea why we are using a child process here. It seems more
complicated than it should. But given this is indeed needed, the fix
seems correct.
Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory
2024-04-12 14:15 ` Lucas De Marchi
@ 2024-04-12 16:20 ` Zbigniew Kempczyński
0 siblings, 0 replies; 5+ messages in thread
From: Zbigniew Kempczyński @ 2024-04-12 16:20 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: igt-dev
On Fri, Apr 12, 2024 at 09:15:43AM -0500, Lucas De Marchi wrote:
> On Fri, Apr 12, 2024 at 02:20:14PM +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>
>
> not idea why we are using a child process here. It seems more
> complicated than it should. But given this is indeed needed, the fix
> seems correct.
What do you mean child process here? I haven't found spawning in
the code.
--
Zbigniew
>
>
> Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Lucas De Marchi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory
2024-04-12 12:20 [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory Zbigniew Kempczyński
2024-04-12 14:15 ` Lucas De Marchi
@ 2024-04-15 9:49 ` Kamil Konieczny
2024-04-15 16:08 ` Janusz Krzysztofik
1 sibling, 1 reply; 5+ messages in thread
From: Kamil Konieczny @ 2024-04-15 9:49 UTC (permalink / raw)
To: igt-dev; +Cc: Zbigniew Kempczyński, Lucas De Marchi, Janusz Krzysztofik
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");
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory
2024-04-15 9:49 ` [i-g-t] " Kamil Konieczny
@ 2024-04-15 16:08 ` Janusz Krzysztofik
0 siblings, 0 replies; 5+ messages in thread
From: Janusz Krzysztofik @ 2024-04-15 16:08 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev, Zbigniew Kempczyński,
Lucas De Marchi, 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@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
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@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");
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-15 16:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-12 12:20 [PATCH i-g-t] lib/igt_kmod: Fix sigaction write to uninitialized memory Zbigniew Kempczyński
2024-04-12 14:15 ` Lucas De Marchi
2024-04-12 16:20 ` Zbigniew Kempczyński
2024-04-15 9:49 ` [i-g-t] " Kamil Konieczny
2024-04-15 16:08 ` Janusz Krzysztofik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox