On 1 July 2015 at 17:56, Ilia Mirkin imirkin@alum.mit.edu wrote:
On Wed, Jul 1, 2015 at 12:51 PM, Colin King colin.king@canonical.com wrote:
From: Colin Ian King colin.king@canonical.com
Various usif_ioctl helper functions do not initialize the return variable ret and some of the error handling return paths just return garbage values that were on the stack (or in a register). I believe that in all the cases, the initial ret variable should be set to -EINVAL and subsequent paths through these helper functions set it appropriately otherwise.
Found via static analysis using cppcheck:
[drivers/gpu/drm/nouveau/nouveau_usif.c:138]: (error) Uninitialized variable: ret
It sure would seem that way, wouldn't it?
#define nvif_unpack(d,vl,vh,m) ({ \ if ((vl) == 0 || ret == -ENOSYS) { \ int _size = sizeof(d); \ if (_size <= size && (d).version >= (vl) && \ (d).version <= (vh)) { \ data = (u8 *)data + _size; \ size = size - _size; \ ret = ((m) || !size) ? 0 : -E2BIG; \ } else { \ ret = -ENOSYS; \ } \ } \ (ret == 0); \ })
So actually it does get initialized, and I guess cppcheck doesn't know about macros?
I think I'm having deja-vu, but I do recall a similar mention to Ben. Although in my defence I've assumed that nvif_unpack was a function, as macros normally are normally all caps. Seems like the patch that capitalises nvif_unpack never made it upstream :'-(
Cheers, Emil