Re: [RFC][PATCH 1/6] RTC subsystem, class

[Russell King <rmk+lkml@arm.linux.org.uk>]

On Tue, Dec 20, 2005 at 09:13:45PM +0000, Christoph Hellwig wrote:
> > +int rtc_read_time(struct class_device *class_dev, struct rtc_time *tm)
> > +{
> > +	int err = -EINVAL;
> > +	struct rtc_class_ops *ops = class_get_devdata(class_dev);
> > +
> > +	if (ops->read_time) {
> > +		memset(tm, 0, sizeof(struct rtc_time));
> 
> do we really need the memset?

Absolutely yes, otherwise if 'tm' is on the stack and it ultimately
gets copied to userspace, it will leak kernel memory.  Why?
Unfortunately, not all elements of 'tm' are written to by RTC
drivers.

You can argue that the RTC drivers need fixing, but since this bug
has gone completely unnoticed in _all_ kernels which have an RTC
driver up until I discovered it and reported it to vendor-sec
during the 2.5 cycle, I think a little bit of cheap protection
against buggy drivers when security leaks are concerned is not
unreasonable.  Especially when they don't get found in the normal
run of things.

(you could make a case for eliminating it _if_ there was a RTC
subsystem maintainer who knew the code and therefore knew what
to look out for.)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core