Re: [RFC] [PATCH] DRM TTM Memory Manager patch

[Thomas Hellström]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/