Messages in this thread | | | Date | Thu, 6 Apr 2006 02:05:09 +0100 | From | Al Viro <> | Subject | Re: [patch 03/26] sysfs: zero terminate sysfs write buffers (CVE-2006-1055) |
| |
On Wed, Apr 05, 2006 at 03:58:15PM -0400, Valdis.Kletnieks@vt.edu wrote: > On Wed, 05 Apr 2006 16:39:57 BST, Al Viro said: > > > How about _NOT_ using sysfs and just having ->read()/->write() on a file in fs > > of your own? ~20 lines for all of it, not counting #include... > > Great. Instead of everybody using the same piece-of-manure sysfs interface, > each driver carries around its 20 lines to implement read() and write() in > subtly buggy and incompatible ways.
No, that would be 20 lines to tell what and where you want in that fs and how long should the things live. Plus whatever you've got for your ->read() and ->write() - using existing libfs helpers if needed. Instead of pushing into sysfs the things that do not fit sysfs interfaces.
BTW, in my experience "subtly buggy and incompatible ways" describes sysfs uses, except that there's rarely anything subtle about that. Care to name four kernel data structures that got kobjects embedded into them (directly or via struct device and it ilk) and had _NOT_ required at one point or another (post-merge) fixing of blatant user-exploitable holes due to botched lifetime rules?
Not that you had to embed them to achieve the same wonderful effect - witness fbsysfs.c user-exploitable holes on unregister_framebuffer(); sure, fb_info->class_device will stay allocated if you have one of the attributes opened. Now try to call read(); what will it access?
Not to mention that the same file has a pile of ->store() assuming we have NUL-termination, or the lovely use of sscanf() on non-NUL-terminated array right in store_cmap() itself. Equivalent of p = malloc(5); if (p) { memcpy(p, q, 5); sscanf(p, "%4hx", &v); } You do realize that it's broken, don't you? sscanf field width for %x applies _after_ skipping the whitespace, not to the total amount of characters being eaten. And in reality this buffer comes from the end of get_zeroed_page() result, so there's really nothing past its end. - 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/
| |