On 3/4/26 13:32, Julian Orth wrote:
> On Wed, Mar 4, 2026 at 12:47 PM Michel Dänzer
> <[email protected]> wrote:
>> On 3/4/26 12:25, Julian Orth wrote:
>>> On Wed, Mar 4, 2026 at 12:15 PM Michel Dänzer
>>> <[email protected]> wrote:
>>>> On 3/3/26 20:12, Julian Orth wrote:
>>>>> On Tue, Mar 3, 2026 at 7:53 PM Michel Dänzer <[email protected]>
>>>>> wrote:
>>>>>> On 3/3/26 18:44, Maarten Lankhorst wrote:
>>>>>>>
>>>>>>> You don't even need to use memset, this would work too:
>>>>>>>
>>>>>>> struct drm_syncobj_handle args = {
>>>>>>> .flags = 0
>>>>>>> };
>>>>>>
>>>>>> TL;DR: This method isn't 100% safe either.
>>>>>>
>>>>>> It won't initialize any padding which isn't covered by any struct field.
>>>>>> We try to avoid that and have explicit padding fields instead, mistakes
>>>>>> may happen though, and in theory such padding could later be used for a
>>>>>> new field.
>>>>>
>>>>> I don't think this is workable.
>>>>
>>>> libdrm begs to differ. It shows that it's not only workable but really
>>>> easy. There's no reason for doing it any other way.
>>>
>>> Using memset to initialize padding bytes between fields is workable.
>>> Having the kernel add checks for this for existing ioctls is not
>>> workable because it would break usespace that doesn't do this.
>>
>> As discussed in this thread, memset is also required for when the size of an
>> ioctl struct is extended, even if there is no such padding.
>
> That is not a concern in rust code. If the rust code extends the
> definition of the struct, all call sites will cause a compilation
> error until the new field is also initialized.
>
> The issue I'm talking about here is strictly about padding bytes between
> fields.
So this is not related to the issue which motivated your patch?
>>> Which is every rust program out there as far as I can tell.
>>
>> That's surprising. Surely there must be some unsafe code involved which
>> allows uninitialized memory to be passed to ioctl()?
>
> The memory is not uninitialized from the perspective of the rust
> language since all fields are initialized. Only the padding bytes are
> uninitialized and they cannot be accessed in safe rust, therefore no
> unsafe is required.
>
> I've never seen any rust code that uses memset to initialize ioctl
> arguments. It assumes that the ioctl implementation will only read
> from the named fields. Therefore, while the ioctl syscall itself is
> unsafe, developers assume that the safety requirements are satisfied
> in this regard.
As discussed in this thread, that's not a valid assumption and may blow up in
their face sooner or later. ("No padding not covered by any fields" is
best-effort, not a guarantee)
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
