linux-kernel - Re: [PATCH] fix RTC_CLASS regression with PARISC

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:	Mon, 8 Sep 2008 20:17:23 -0700
From:	David Brownell <david-b@...bell.net>
To:	David Miller <davem@...emloft.net>
Cc:	James.Bottomley@...senpartnership.com,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, linux-parisc@...r.kernel.org,
	alessandro.zummo@...ertech.it
Subject: Re: [PATCH] fix RTC_CLASS regression with PARISC

On Monday 08 September 2008, David Miller wrote:
> 
> > > int update_persistent_clock(struct timespec now)
> > > {
> > >     struct rtc_device *rtc = rtc_class_open("rtc0");

One more point:  that should probably use CONFIG_RTC_HCTOSYS_DEVICE
instead of hard-wiring to "rtc0".  Yeah, I'm sure your SPARCs have
lots of RTCs to choose from -- not! -- but I'd like to see you end
up with code that many folk can reuse/recycle/pirate.  ;)

> > 
> > I'd be tempted to cache that ... notice how you never
> > close it, too.  That will goof lots of refcounts...
> 
> Well if I cache it then we'll hold it forever and that's not
> so nice right?

Why wouldn't it be, so long as it's eventually closed
to prevent leakage?  Other code can rtc_class_open() too;
unlike a userspace open("/dev/rtc0", ...) this isn't an
exclusive operation.

> I'm going to put the missing rtc_close() in there for now to
> fix the leak.
> 
> I'm happy to cache this if you think it's warranted, but then
> this is like saying that the refcount doesn't matter :-)

If you're concerned about stuff like "rmmod my-i2c-rtc-driver"
losing (or "rmmod my-i2c-rtc-driver's-i2c-adapter") ... what's
supposed to happen is that you start getting an -ENODEV return
from your rtc_set_mmss() call, and then you close and null your
cached handle to free up its memory.

The next time you go through this routine you'd see nothing
is cached and then try to get an RTC.  Maybe it's available
again; maybe not.

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