Messages in this thread | | | Date | Tue, 01 May 2007 02:10:48 +0200 | From | Thomas Hellström <> | Subject | Re: [RFC] [PATCH] DRM TTM Memory Manager patch |
| |
Dave Airlie wrote:
> > Most likely in doxygen as that is what Mesa uses and the intersection > of developers is higher in that area, I'll take it as a task to try > and kerneldoc the drm at some stage.. > >> - what's with the /proc interface? Don't add new proc code for >> non-process related things. This should all go into sysfs >> somewhere. And yes, I know /proc/dri/ is there today, but don't add >> new stuff please. > > > Well we should move all that stuff to sysfs, but we have all the > infrastructure for publishing this stuff under /proc/dri and adding > new files doesn't take a major amount, as much as I appreciate sysfs, > it isn't suitable for this sort of information dump, the whole one > value per file is quite useless to provide this sort of information > which is uni-directional for users to send to us for debugging without > have to install some special tool to join all the values into one > place.. and I don't think drmfs is the answer either... or maybe it > is.... > >> - struct drm_bo_arg can't use an int or unsigned, as it crosses the >> userspace/kernelspace boundry, use the proper types for all values >> in those types of structures (__u32, etc.) > > > int is defined, unsigned I'm not so sure about, the drm user space > interface is usually specified in non-system specific types so the > drm.h file is consistent across systems, so we would probably have to > use uint32_t which other people have objected to, but I'd rather use > uint32_t than unspecified types.. > >> - there doesn't seem to be any validity checking for the arguments >> passed into this new ioctl. Possibly that's just the way the rest >> of the dri interface is, which is scary, but with the memory stuff, >> you really should check things properly... > > > Okay this needs fixing, we do check most ioctls args, the main thing > passed in are handles and these are all looked up in the hash table, > it may not be so obvious, also most of the ioctls are probably going > to end up root or DRM master only, I'd like do an ioctl fuzzer at some > stage, I'd suspect a lot more then the dri would be oopsable with > permissions... > > Thanks, > Dave.
I agree with Dave for most if not all of the above. Typing, for example unsigned / uint32 vs u32, __u32 is very easily fixable once we decide on a clear way to go to keep (if we want to keep) compatibility with *bsd.
For the IOCTL checking, as Dave states, most invalid data will be trapped in hash lookups and checks in the buffer validation system, but probably far from all. A fuzzer would be a nice tool to trap the exceptions.
/Thomas
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |