[Public] Hi,
I studied the documentation at https://docs.kernel.org/process/botching-up-ioctls.html In this case there is size and a pointer, so it is not strictly required. But good to follow coding guidelines. @Liang, Prike<mailto:[email protected]> your proposed changes looks good. Thank you, Yogesh ________________________________ From: Liang, Prike <[email protected]> Sent: Friday, March 20, 2026 8:11 AM To: Michel Dänzer <[email protected]>; Koenig, Christian <[email protected]>; Mohan Marimuthu, Yogesh <[email protected]>; Khatri, Sunil <[email protected]>; Zhang, Jesse(Jie) <[email protected]>; Deucher, Alexander <[email protected]>; Olsak, Marek <[email protected]> Cc: [email protected] <[email protected]> Subject: RE: [PATCH] Revert "drm/amdgpu: harden SIGNAL/WAIT ioctl argument validation" [Public] Yes, in this case it’s cleaner and more robust to not allocate at all when num == 0, and keep the pointer as NULL. For the mesa driver, how about use the following allocation pattern? unsigned num_syncobj_dependencies = csc->syncobj_dependencies.num; uint32_t *syncobj_dependencies_list = NULL; if (num_syncobj_dependencies > 0) { syncobj_dependencies_list = alloca(num_syncobj_dependencies * sizeof(uint32_t)); /* fill the buffer */ } Regards, Prike > -----Original Message----- > From: Michel Dänzer <[email protected]> > Sent: Thursday, March 19, 2026 6:47 PM > To: Koenig, Christian <[email protected]>; Liang, Prike > <[email protected]>; Mohan Marimuthu, Yogesh > <[email protected]>; Khatri, Sunil <[email protected]>; > Zhang, Jesse(Jie) <[email protected]>; Deucher, Alexander > <[email protected]>; Olsak, Marek <[email protected]> > Cc: [email protected] > Subject: Re: [PATCH] Revert "drm/amdgpu: harden SIGNAL/WAIT ioctl argument > validation" > > On 3/19/26 08:30, Christian König wrote: > > Hi guys, > > > > well when mesa leaves some fields in the structure uninitialized then that > > is a > pretty bad idea and we should eventually fix that. > > > > But always setting the pointers to valid arrays and just setting the number > > of array > elements to zero is perfectly valid. > > > > That doesn't even needs a debug message. > > As discussed recently for another patch, the "(How to avoid) Botching up > ioctls" > page of the kernel documentation says under Basics: > > * Check all unused fields and flags and all the padding for whether it’s 0, > and reject > the ioctl if that’s not the case. > > That seems to apply here, i.e. the kernel should have these checks and Mesa > should > initialize the pointer field to 0 when the corresponding num_* field is. > > > P.S. I agree it probably doesn't make a practical difference in this specific > case. I > suspect the rule is aimed at when the ioctl struct is extended, in which case > Mesa's > current behaviour would be indistinguishable from user-space code which > actually > doesn't properly initialize the newly-added fields. > > It seems safer to stick to the rule even in cases like this where it's not > strictly > required. > > > -- > Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer > https://redhat.com \ Libre software enthusiast
