Re: [PATCH] ptrace: add PTRACE_GET_RSEQ_CONFIGURATION request

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Feb 26, 2021, at 9:11 AM, Piotr Figiel figiel@google.com wrote:

> Hi,
> 
> On Mon, Feb 22, 2021 at 09:53:17AM -0500, Mathieu Desnoyers wrote:
> 
>> I notice that other structures defined in this UAPI header are not
>> packed as well.  Should we add an attribute packed on new structures ?
>> It seems like it is generally a safer course of action, even though
>> each field is naturally aligned here (there is no padding/hole in the
>> structure).
> 
> I considered this for quite a while. There are some gains for this
> approach, i.e. it's safer towards the ISO C, as theoretically compiler
> can generate arbitrary offsets as long as struct elements have correct
> order in memory.
> Also with packed attribute it would be harder to make it incorrect in
> future modifications.
> User code also could theoretically put the structure on any misaligned
> address.
> 
> But the drawback is that all accesses to the structure contents are
> inefficient and some compilers may generate large chunks of code
> whenever the structure elements are accessed (I recall at least one ARM
> compiler which generates series of single-byte accesses for those). For
> kernel it doesn't matter much because the structure type is used in one
> place, but it may be different for the application code.
> 
> The change would be also inconsistent with the rest of the file and IMO
> the gains are only theoretical.
> 
> If there are more opinions on this or you have some argument I'm missing
> please let me know I can send v3 with packed and explicit padding
> removed. I think this is rather borderline trade off.

I personally don't have a strong opinion on this and completely agree with
your analysis. Maybe for pre-existing system calls adding more non-packed
structures might be kind-of OK if some were already exposed, even though
it seems rather fragile wrt ISO C.

Thanks,

Mathieu

> 
> Best regards and thanks for looking at this,
> Piotr.

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com