Re: PATCH: 2.6.7-rc3 drivers/video/fbmem.c: user/kernel pointer bugs

[Robert T. Johnson]

On Wed, 2004-06-09 at 21:15, viro@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
wrote:
> On Wed, Jun 09, 2004 at 08:50:33PM -0700, Robert T. Johnson wrote:
> > static int pty_write(struct tty_struct * tty, int from_user,
> > 		       const unsigned char __user   *ubuf, 
> >                        const unsigned char __kernel *kbuf,
> >                        int count)
> 
> So I suspect that it in the long run the proper fix will be to sanitize
> the locking and move all copy_from_user() to the (only) caller.

I agree this is the ideal fix.  I can see advantages and disadvantages
to all the approaches.  I'm not familiar with the locking issues, so I
can't comment on that.

> > I fear that completely separating ioctl and kernel data structures would
> > result in lots of redundant structure definitions, which will lead to
> > code maintainence problems and their own host of bugs.  Would it be
> > better to just design a bug finding tool that's capable of keeping track
> > of different structure instances separately?
> 
> I doubt it.  Most of the ioctl data structures do not survive past the
> decoding; fb layer is ugly that way, but that's a local problem and it
> can be fixed.
> 
> Keep in mind that anything containing userland pointers needs to be explicitly
> dealt with on 32/64 platforms - otherwise 32bit code won't be able to issue
> that ioctl anyway.
> 
> 	Besides, kernel data structures should not be tied by ABI stability
> requirements - and ioctl arguments have to.  Which leads to far worse bug
> potential than explict decoding.

These are design issues outside the scope of what I'm doing, but they
are important.  I'll try to keep these considerations in mind as I
continue to improve cqual.  Thanks for the helpful feedback.

Best,
Rob


-
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/