exec.c: Initialize sa_flags passed to sigaction()

Message ID 20180515182700.31736-1-peter.maydell@linaro.org
State New
Headers show
Series
  • exec.c: Initialize sa_flags passed to sigaction()
Related show

Commit Message

Peter Maydell May 15, 2018, 6:27 p.m.
Coverity points out that in the user-only version of cpu_abort() we
call sigaction() with a partially initialized struct sigaction
(CID 1005351). Correct the omission.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 exec.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.17.0

Comments

Alex Bennée May 15, 2018, 7:24 p.m. | #1
Peter Maydell <peter.maydell@linaro.org> writes:

> Coverity points out that in the user-only version of cpu_abort() we

> call sigaction() with a partially initialized struct sigaction

> (CID 1005351). Correct the omission.

>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---

>  exec.c | 1 +

>  1 file changed, 1 insertion(+)

>

> diff --git a/exec.c b/exec.c

> index ffa1099547..bd8833fc9d 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -1124,6 +1124,7 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)

>          struct sigaction act;

>          sigfillset(&act.sa_mask);

>          act.sa_handler = SIG_DFL;

> +        act.sa_flags = 0;

>          sigaction(SIGABRT, &act, NULL);

>      }

>  #endif



--
Alex Bennée
Philippe Mathieu-Daudé May 15, 2018, 8:53 p.m. | #2
On 05/15/2018 03:27 PM, Peter Maydell wrote:
> Coverity points out that in the user-only version of cpu_abort() we

> call sigaction() with a partially initialized struct sigaction

> (CID 1005351). Correct the omission.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  exec.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/exec.c b/exec.c

> index ffa1099547..bd8833fc9d 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -1124,6 +1124,7 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)

>          struct sigaction act;


I'd have used the more generic:

           struct sigaction act = { };

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


>          sigfillset(&act.sa_mask);

>          act.sa_handler = SIG_DFL;

> +        act.sa_flags = 0;

>          sigaction(SIGABRT, &act, NULL);

>      }

>  #endif

>
Eric Blake May 15, 2018, 9:40 p.m. | #3
On 05/15/2018 03:53 PM, Philippe Mathieu-Daudé wrote:
> On 05/15/2018 03:27 PM, Peter Maydell wrote:

>> Coverity points out that in the user-only version of cpu_abort() we

>> call sigaction() with a partially initialized struct sigaction

>> (CID 1005351). Correct the omission.

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>>   exec.c | 1 +

>>   1 file changed, 1 insertion(+)

>>

>> diff --git a/exec.c b/exec.c

>> index ffa1099547..bd8833fc9d 100644

>> --- a/exec.c

>> +++ b/exec.c

>> @@ -1124,6 +1124,7 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)

>>           struct sigaction act;

> 

> I'd have used the more generic:

> 

>             struct sigaction act = { };


That's a gcc/clang extension (although we have used it before, 
particularly to shut up buggy versions of clang); better is:

struct sigaction act = { 0 };

if that doesn't trigger the clang bug.

> 

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 

>>           sigfillset(&act.sa_mask);

>>           act.sa_handler = SIG_DFL;


The sigfillset() has to be done after initialization, but you could also 
use:

struct sigaction act = {
   .sa_handler = SIG_DFL;
};
sigfillset(&act.sa_mask);

as a way to zero-initialize all other fields.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Paolo Bonzini May 16, 2018, 7:25 a.m. | #4
On 15/05/2018 20:27, Peter Maydell wrote:
> Coverity points out that in the user-only version of cpu_abort() we

> call sigaction() with a partially initialized struct sigaction

> (CID 1005351). Correct the omission.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  exec.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/exec.c b/exec.c

> index ffa1099547..bd8833fc9d 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -1124,6 +1124,7 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)

>          struct sigaction act;

>          sigfillset(&act.sa_mask);

>          act.sa_handler = SIG_DFL;

> +        act.sa_flags = 0;

>          sigaction(SIGABRT, &act, NULL);

>      }

>  #endif

> 


Queued, thanks.

Paolo

Patch

diff --git a/exec.c b/exec.c
index ffa1099547..bd8833fc9d 100644
--- a/exec.c
+++ b/exec.c
@@ -1124,6 +1124,7 @@  void cpu_abort(CPUState *cpu, const char *fmt, ...)
         struct sigaction act;
         sigfillset(&act.sa_mask);
         act.sa_handler = SIG_DFL;
+        act.sa_flags = 0;
         sigaction(SIGABRT, &act, NULL);
     }
 #endif