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 21:14:00 -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:
> > > > >     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.  ;)
> 
> Can you be more specific?  Oh, you want me to use the string defined
> by that config option.  Ok :-)
> 
> But as far as I can tell this will only be set of RTC_HCTOSYS and
> users currently are allowed to not set that.
> 
> If this code goes somewhere generic you would need to ifdef test on
> that, depending upon where you'd want to put it and how it would
> be provided generically.

OK.


> > > > 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.
> 
> When would be "eventually closed" if I open it here and remember
> the pointer in a static local variable, and don't close it?
> 
> I guess you need to be more specific about what you mean by
> caching :)

I'll translate that as "-ENOPATCH".  :)

It'd suffice to fire a timer every 15 minutes or so, and
close it if the NTP logic hasn't refreshed the clock since
last time.  You're right that the simplest scheme is to just
open/close on each call.  The extra work is so infrequent it
may not be measurable.


> > 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.
> 
> I see... god that's ugly.  If you want to do this in the generic
> RTC layer helper routines,

... when they get created ...


> 	that's fine, but I don't feel like 
> adding all sorts of stuff like that to the sparc specific routine
> at the moment.
> 
> I'm trying to do things that are practical and that I can check
> into sparc-next-2.6 right now.

OK by me.



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