linux-kernel - Re: [RFC] [PATCH] DRM TTM Memory Manager patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 01 May 2007 02:10:48 +0200
From:	Thomas Hellström <thomas@...gstengraphics.com>
To:	Dave Airlie <airlied@...il.com>
CC:	Greg KH <greg@...ah.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Eric Anholt <eric@...olt.net>
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/