On Wed, Mar 4, 2026 at 3:30 PM Michel Dänzer <[email protected]> wrote:
>
> 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?
I don't believe the ioctls in question contain padding bytes on any
architecture. This thread is in response to your statement above:
>>>>>>> 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.
----
>
>
> >>> 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
