[PATCH] um: Fetch registers only for signals which need them

[Anton Ivanov]

On 10/11/2020 11:27, Johannes Berg wrote:
> On Tue, 2020-11-10 at 11:11 +0000, anton.ivanov at cambridgegreys.com
> wrote:
>> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
>>
>> UML userspace fetches siginfo and passes it to signal handlers
>> in UML. This is needed only for some of the signals, because
>> key handlers like SIGIO make no use of this variable.
> 
> It looks like even for SIGSEGV it depends on PTRACE_FULL_FAULTINFO,
> which is only set for 64-bit?
> 
> 
>> -			ptrace(PTRACE_GETSIGINFO, pid, 0, (struct siginfo *)&si);
>> +			switch (sig) {
>> +			case SIGSEGV:
>> +			case SIGTRAP:
>> +			case SIGILL:
>> +			case SIGBUS:
>> +			case SIGFPE:
>> +			case SIGWINCH:
>> +				ptrace(PTRACE_GETSIGINFO, pid, 0, (struct siginfo *)&si);
>> +				break;
>> +			}
>>   
>>   			switch (sig) {
>>   			case SIGSEGV:
> 
> So perhaps it might make sense to push it further down into that switch?
> It'd end up duplicating it to three places though.
> 
> In the SIGIO case you could write it as
> 
> case SIGILL:
> case SIGBUS:
> case SIGFPE:
> case SIGWINCH:
> 	ptrace(...);
> 	fallthrough;

gcc now complains loudly about fallthrough cases. Though I can override it, I'd rather leave the original approach where there is a list of SIGs which need the si fetched.

> case SIGIO:
> 	block_signals_trace();
> 	// etc
> 
> But I have no strong opinion either way really.
> 
> 
> I do think a comment is needed though, because we _do_ pass the pointer

I added a comment to v2 which explains what and why is being done.

> to the sigio handler ... or maybe separate that out just like
> handle_trap() etc.?
> 
> johannes
> 
> 

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/